Commit 966ca923 authored by Steve French's avatar Steve French Committed by Linus Torvalds

[PATCH] cifs: Fix caching problem

pointed out by Dave Stahl and Vince Negri in which cifs can update the
last modify time on a server modified file without invalidating the
local cached data due to an intervening readdir. 

Signed-off-by: Steve French (sfrench@us.ibm.com)
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 433dc24f
...@@ -4,13 +4,16 @@ Steve French (sfrench@samba.org) ...@@ -4,13 +4,16 @@ Steve French (sfrench@samba.org)
The author wishes to express his appreciation and thanks to: The author wishes to express his appreciation and thanks to:
Andrew Tridgell (Samba team) for his early suggestions about smb/cifs VFS Andrew Tridgell (Samba team) for his early suggestions about smb/cifs VFS
improvements. Thanks to IBM for allowing me the time and test resources to pursue improvements. Thanks to IBM for allowing me time and test resources to pursue
this project. Jim McDonough from IBM (and the Samba Team) for his help. this project, to Jim McDonough from IBM (and the Samba Team) for his help, to
The IBM Linux JFS team for explaining many esoteric Linux filesystem features. the IBM Linux JFS team for explaining many esoteric Linux filesystem features.
Jeremy Allison of the Samba team has done invaluable work in adding the server
side of the original CIFS Unix extensions and reviewing and implementing
portions of the newer CIFS POSIX extensions into the Samba 3 file server. Thank
Dave Boutcher of IBM Rochester (author of the OS/400 smb/cifs filesystem client) Dave Boutcher of IBM Rochester (author of the OS/400 smb/cifs filesystem client)
for proving years ago that a very good smb/cifs client could be done on a Unix like for proving years ago that very good smb/cifs clients could be done on Unix-like
operating system. Volker Lendecke, Andrew Tridgell, Urban Widmark, John Newbigin operating systems. Volker Lendecke, Andrew Tridgell, Urban Widmark, John
and others for their work on the Linux smbfs module over the years. Thanks to Newbigin and others for their work on the Linux smbfs module. Thanks to
the other members of the Storage Network Industry Association CIFS Technical the other members of the Storage Network Industry Association CIFS Technical
Workgroup for their work specifying this highly complex protocol and finally Workgroup for their work specifying this highly complex protocol and finally
thanks to the Samba team for their technical advice and encouragement. thanks to the Samba team for their technical advice and encouragement.
...@@ -24,9 +27,11 @@ Shobhit Dayal ...@@ -24,9 +27,11 @@ Shobhit Dayal
Sergey Vlasov Sergey Vlasov
Richard Hughes Richard Hughes
Yury Umanets Yury Umanets
Mark Hamzy Mark Hamzy (for some of the early cifs IPv6 work)
Domen Puncer Domen Puncer
Jesper Juhl Jesper Juhl (in particular for lots of whitespace/formatting cleanup)
Vince Negri and Dave Stahl (for finding an important caching bug)
Adrian Bunk (kcalloc cleanups)
Test case and Bug Report contributors Test case and Bug Report contributors
------------------------------------- -------------------------------------
...@@ -36,7 +41,8 @@ Rene Scharfe, Martin Josefsson, Alexander Wild, Anthony Liguori, ...@@ -36,7 +41,8 @@ Rene Scharfe, Martin Josefsson, Alexander Wild, Anthony Liguori,
Lars Muller, Urban Widmark, Massimiliano Ferrero, Howard Owen, Lars Muller, Urban Widmark, Massimiliano Ferrero, Howard Owen,
Olaf Kirch, Kieron Briggs, Nick Millington and others. Also special Olaf Kirch, Kieron Briggs, Nick Millington and others. Also special
mention to the Stanford Checker (SWAT) which pointed out many minor mention to the Stanford Checker (SWAT) which pointed out many minor
bugs in error paths. bugs in error paths. Valuable suggestions also have come from Al Viro
and Dave Miller.
And thanks to the IBM LTC and Power test teams and SuSE testers for And thanks to the IBM LTC and Power test teams and SuSE testers for
finding multiple bugs during excellent stress test runs. finding multiple bugs during excellent stress test runs.
Version 1.33
------------
Fix caching problem, in which readdir of directory containing a file
which was cached could cause the file's time stamp to be updated
without invalidating the readahead data (so we could get stale
file data on the client for that file even as the server copy changed).
Version 1.32 Version 1.32
------------ ------------
Fix oops in ls when Transact2 FindFirst (or FindNext) returns more than one Fix oops in ls when Transact2 FindFirst (or FindNext) returns more than one
......
/* /*
* fs/cifs/cifsfs.h * fs/cifs/cifsfs.h
* *
* Copyright (c) International Business Machines Corp., 2002 * Copyright (c) International Business Machines Corp., 2002, 2005
* Author(s): Steve French (sfrench@us.ibm.com) * Author(s): Steve French (sfrench@us.ibm.com)
* *
* This library is free software; you can redistribute it and/or modify * This library is free software; you can redistribute it and/or modify
...@@ -96,5 +96,5 @@ extern ssize_t cifs_getxattr(struct dentry *, const char *, void *, size_t); ...@@ -96,5 +96,5 @@ extern ssize_t cifs_getxattr(struct dentry *, const char *, void *, size_t);
extern ssize_t cifs_listxattr(struct dentry *, char *, size_t); extern ssize_t cifs_listxattr(struct dentry *, char *, size_t);
extern int cifs_ioctl (struct inode * inode, struct file * filep, extern int cifs_ioctl (struct inode * inode, struct file * filep,
unsigned int command, unsigned long arg); unsigned int command, unsigned long arg);
#define CIFS_VERSION "1.32" #define CIFS_VERSION "1.33"
#endif /* _CIFSFS_H */ #endif /* _CIFSFS_H */
...@@ -464,7 +464,7 @@ CIFSSMBTDis(const int xid, struct cifsTconInfo *tcon) ...@@ -464,7 +464,7 @@ CIFSSMBTDis(const int xid, struct cifsTconInfo *tcon)
rc = SendReceive(xid, tcon->ses, smb_buffer, smb_buffer_response, rc = SendReceive(xid, tcon->ses, smb_buffer, smb_buffer_response,
&length, 0); &length, 0);
if (rc) if (rc)
cFYI(1, (" Tree disconnect failed %d", rc)); cFYI(1, ("Tree disconnect failed %d", rc));
if (smb_buffer) if (smb_buffer)
cifs_small_buf_release(smb_buffer); cifs_small_buf_release(smb_buffer);
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
* *
* Directory search handling * Directory search handling
* *
* Copyright (C) International Business Machines Corp., 2004 * Copyright (C) International Business Machines Corp., 2004, 2005
* Author(s): Steve French (sfrench@us.ibm.com) * Author(s): Steve French (sfrench@us.ibm.com)
* *
* This library is free software; you can redistribute it and/or modify * This library is free software; you can redistribute it and/or modify
...@@ -65,14 +65,14 @@ static int construct_dentry(struct qstr *qstring, struct file *file, ...@@ -65,14 +65,14 @@ static int construct_dentry(struct qstr *qstring, struct file *file,
struct cifsTconInfo *pTcon; struct cifsTconInfo *pTcon;
int rc = 0; int rc = 0;
cFYI(1, ("For %s ", qstring->name)); cFYI(1, ("For %s", qstring->name));
cifs_sb = CIFS_SB(file->f_dentry->d_sb); cifs_sb = CIFS_SB(file->f_dentry->d_sb);
pTcon = cifs_sb->tcon; pTcon = cifs_sb->tcon;
qstring->hash = full_name_hash(qstring->name, qstring->len); qstring->hash = full_name_hash(qstring->name, qstring->len);
tmp_dentry = d_lookup(file->f_dentry, qstring); tmp_dentry = d_lookup(file->f_dentry, qstring);
if (tmp_dentry) { if (tmp_dentry) {
cFYI(0, (" existing dentry with inode 0x%p", tmp_dentry->d_inode)); cFYI(0, ("existing dentry with inode 0x%p", tmp_dentry->d_inode));
*ptmp_inode = tmp_dentry->d_inode; *ptmp_inode = tmp_dentry->d_inode;
/* BB overwrite old name? i.e. tmp_dentry->d_name and tmp_dentry->d_name.len??*/ /* BB overwrite old name? i.e. tmp_dentry->d_name and tmp_dentry->d_name.len??*/
if(*ptmp_inode == NULL) { if(*ptmp_inode == NULL) {
...@@ -105,8 +105,11 @@ static int construct_dentry(struct qstr *qstring, struct file *file, ...@@ -105,8 +105,11 @@ static int construct_dentry(struct qstr *qstring, struct file *file,
} }
static void fill_in_inode(struct inode *tmp_inode, static void fill_in_inode(struct inode *tmp_inode,
FILE_DIRECTORY_INFO *pfindData, int *pobject_type) FILE_DIRECTORY_INFO *pfindData, int *pobject_type, int isNewInode)
{ {
loff_t local_size;
struct timespec local_mtime;
struct cifsInodeInfo *cifsInfo = CIFS_I(tmp_inode); struct cifsInodeInfo *cifsInfo = CIFS_I(tmp_inode);
struct cifs_sb_info *cifs_sb = CIFS_SB(tmp_inode->i_sb); struct cifs_sb_info *cifs_sb = CIFS_SB(tmp_inode->i_sb);
__u32 attr = le32_to_cpu(pfindData->ExtFileAttributes); __u32 attr = le32_to_cpu(pfindData->ExtFileAttributes);
...@@ -116,6 +119,10 @@ static void fill_in_inode(struct inode *tmp_inode, ...@@ -116,6 +119,10 @@ static void fill_in_inode(struct inode *tmp_inode,
cifsInfo->cifsAttrs = attr; cifsInfo->cifsAttrs = attr;
cifsInfo->time = jiffies; cifsInfo->time = jiffies;
/* save mtime and size */
local_mtime = tmp_inode->i_mtime;
local_size = tmp_inode->i_size;
/* Linux can not store file creation time unfortunately so ignore it */ /* Linux can not store file creation time unfortunately so ignore it */
tmp_inode->i_atime = tmp_inode->i_atime =
cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastAccessTime)); cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastAccessTime));
...@@ -134,7 +141,6 @@ static void fill_in_inode(struct inode *tmp_inode, ...@@ -134,7 +141,6 @@ static void fill_in_inode(struct inode *tmp_inode,
tmp_inode->i_mode = cifs_sb->mnt_file_mode; tmp_inode->i_mode = cifs_sb->mnt_file_mode;
} }
cFYI(0,("CIFS FFIRST: Attributes came in as 0x%x",attr));
if (attr & ATTR_DIRECTORY) { if (attr & ATTR_DIRECTORY) {
*pobject_type = DT_DIR; *pobject_type = DT_DIR;
/* override default perms since we do not lock dirs */ /* override default perms since we do not lock dirs */
...@@ -175,30 +181,46 @@ static void fill_in_inode(struct inode *tmp_inode, ...@@ -175,30 +181,46 @@ static void fill_in_inode(struct inode *tmp_inode,
(unsigned long)tmp_inode->i_size, tmp_inode->i_blocks, (unsigned long)tmp_inode->i_size, tmp_inode->i_blocks,
tmp_inode->i_blksize)); tmp_inode->i_blksize));
if (S_ISREG(tmp_inode->i_mode)) { if (S_ISREG(tmp_inode->i_mode)) {
cFYI(1, (" File inode ")); cFYI(1, ("File inode"));
tmp_inode->i_op = &cifs_file_inode_ops; tmp_inode->i_op = &cifs_file_inode_ops;
if(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DIRECT_IO) if(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DIRECT_IO)
tmp_inode->i_fop = &cifs_file_direct_ops; tmp_inode->i_fop = &cifs_file_direct_ops;
else else
tmp_inode->i_fop = &cifs_file_ops; tmp_inode->i_fop = &cifs_file_ops;
tmp_inode->i_data.a_ops = &cifs_addr_ops; tmp_inode->i_data.a_ops = &cifs_addr_ops;
if(isNewInode)
return; /* No sense invalidating pages for new inode since we
have not started caching readahead file data yet */
if (timespec_equal(&tmp_inode->i_mtime, &local_mtime) &&
(local_size == tmp_inode->i_size)) {
cFYI(1, ("inode exists but unchanged"));
} else {
/* file may have changed on server */
cFYI(1, ("invalidate inode, readdir detected change"));
invalidate_remote_inode(tmp_inode);
}
} else if (S_ISDIR(tmp_inode->i_mode)) { } else if (S_ISDIR(tmp_inode->i_mode)) {
cFYI(1, (" Directory inode")); cFYI(1, ("Directory inode"));
tmp_inode->i_op = &cifs_dir_inode_ops; tmp_inode->i_op = &cifs_dir_inode_ops;
tmp_inode->i_fop = &cifs_dir_ops; tmp_inode->i_fop = &cifs_dir_ops;
} else if (S_ISLNK(tmp_inode->i_mode)) { } else if (S_ISLNK(tmp_inode->i_mode)) {
cFYI(1, (" Symbolic Link inode ")); cFYI(1, ("Symbolic Link inode"));
tmp_inode->i_op = &cifs_symlink_inode_ops; tmp_inode->i_op = &cifs_symlink_inode_ops;
} else { } else {
cFYI(1, (" Init special inode ")); cFYI(1, ("Init special inode"));
init_special_inode(tmp_inode, tmp_inode->i_mode, init_special_inode(tmp_inode, tmp_inode->i_mode,
tmp_inode->i_rdev); tmp_inode->i_rdev);
} }
} }
static void unix_fill_in_inode(struct inode *tmp_inode, static void unix_fill_in_inode(struct inode *tmp_inode,
FILE_UNIX_INFO *pfindData, int *pobject_type) FILE_UNIX_INFO *pfindData, int *pobject_type, int isNewInode)
{ {
loff_t local_size;
struct timespec local_mtime;
struct cifsInodeInfo *cifsInfo = CIFS_I(tmp_inode); struct cifsInodeInfo *cifsInfo = CIFS_I(tmp_inode);
struct cifs_sb_info *cifs_sb = CIFS_SB(tmp_inode->i_sb); struct cifs_sb_info *cifs_sb = CIFS_SB(tmp_inode->i_sb);
...@@ -208,6 +230,10 @@ static void unix_fill_in_inode(struct inode *tmp_inode, ...@@ -208,6 +230,10 @@ static void unix_fill_in_inode(struct inode *tmp_inode,
cifsInfo->time = jiffies; cifsInfo->time = jiffies;
atomic_inc(&cifsInfo->inUse); atomic_inc(&cifsInfo->inUse);
/* save mtime and size */
local_mtime = tmp_inode->i_mtime;
local_size = tmp_inode->i_size;
tmp_inode->i_atime = tmp_inode->i_atime =
cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastAccessTime)); cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastAccessTime));
tmp_inode->i_mtime = tmp_inode->i_mtime =
...@@ -265,6 +291,19 @@ static void unix_fill_in_inode(struct inode *tmp_inode, ...@@ -265,6 +291,19 @@ static void unix_fill_in_inode(struct inode *tmp_inode,
else else
tmp_inode->i_fop = &cifs_file_ops; tmp_inode->i_fop = &cifs_file_ops;
tmp_inode->i_data.a_ops = &cifs_addr_ops; tmp_inode->i_data.a_ops = &cifs_addr_ops;
if(isNewInode)
return; /* No sense invalidating pages for new inode since we
have not started caching readahead file data yet */
if (timespec_equal(&tmp_inode->i_mtime, &local_mtime) &&
(local_size == tmp_inode->i_size)) {
cFYI(1, ("inode exists but unchanged"));
} else {
/* file may have changed on server */
cFYI(1, ("invalidate inode, readdir detected change"));
invalidate_remote_inode(tmp_inode);
}
} else if (S_ISDIR(tmp_inode->i_mode)) { } else if (S_ISDIR(tmp_inode->i_mode)) {
cFYI(1, ("Directory inode")); cFYI(1, ("Directory inode"));
tmp_inode->i_op = &cifs_dir_inode_ops; tmp_inode->i_op = &cifs_dir_inode_ops;
...@@ -321,7 +360,7 @@ static int initiate_cifs_search(const int xid, struct file *file) ...@@ -321,7 +360,7 @@ static int initiate_cifs_search(const int xid, struct file *file)
return -ENOMEM; return -ENOMEM;
} }
cFYI(1, ("Full path: %s start at: %lld ", full_path, file->f_pos)); cFYI(1, ("Full path: %s start at: %lld", full_path, file->f_pos));
ffirst_retry: ffirst_retry:
/* test for Unix extensions */ /* test for Unix extensions */
...@@ -666,10 +705,15 @@ static int cifs_filldir(char *pfindEntry, struct file *file, ...@@ -666,10 +705,15 @@ static int cifs_filldir(char *pfindEntry, struct file *file,
insert_inode_hash(tmp_inode); insert_inode_hash(tmp_inode);
} }
/* we pass in rc below, indicating whether it is a new inode,
so we can figure out whether to invalidate the inode cached
data if the file has changed */
if(pCifsF->srch_inf.info_level == SMB_FIND_FILE_UNIX) { if(pCifsF->srch_inf.info_level == SMB_FIND_FILE_UNIX) {
unix_fill_in_inode(tmp_inode,(FILE_UNIX_INFO *)pfindEntry,&obj_type); unix_fill_in_inode(tmp_inode,
(FILE_UNIX_INFO *)pfindEntry,&obj_type, rc);
} else { } else {
fill_in_inode(tmp_inode,(FILE_DIRECTORY_INFO *)pfindEntry,&obj_type); fill_in_inode(tmp_inode,
(FILE_DIRECTORY_INFO *)pfindEntry,&obj_type, rc);
} }
rc = filldir(direntry,qstring.name,qstring.len,file->f_pos,tmp_inode->i_ino,obj_type); rc = filldir(direntry,qstring.name,qstring.len,file->f_pos,tmp_inode->i_ino,obj_type);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment