[CIFS] Fix locking problem around some cifs uses of i_size write

Could cause hangs on smp systems in i_size_read on a cifs inode
whose size has been previously simultaneously updated from
different processes.

Thanks to Brian Wang for some great testing/debugging on this
hard problem.

Fixes kernel bugzilla #7903

CC: Shirish Pargoankar <shirishp@us.ibm.com>
CC: Shaggy <shaggy@us.ibm.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>

diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES
index 5fe13593b57faec72a1c9211de31b31f667ae759..e08a147c09e12be99d84fd7dc17411dc7db23c88 100644 (file)
--- a/fs/cifs/CHANGES
+++ b/fs/cifs/CHANGES
@@ -1,3 +1,9 @@
+Verison 1.48
+------------
+Fix mtime bouncing around from local idea of last write times to remote time.
+Fix hang (in i_size_read) when simultaneous size update of same remote file
+on smp system corrupts sequence number.
+ 
 Version 1.47
 ------------
 Fix oops in list_del during mount caused by unaligned string.

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index a1265c9bfec0cb250196dd9acc60bb9a8de76a31..c07ff8317a8b051c4d636cac9d25c49f15bdd5c2 100644 (file)
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -879,18 +879,19 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
        cifs_stats_bytes_written(pTcon, total_written);
 
        /* since the write may have blocked check these pointers again */
-       if (file->f_path.dentry) {
-               if (file->f_path.dentry->d_inode) {
-                       struct inode *inode = file->f_path.dentry->d_inode;
-                       inode->i_ctime = inode->i_mtime =
-                               current_fs_time(inode->i_sb);
-                       if (total_written > 0) {
-                               if (*poffset > file->f_path.dentry->d_inode->i_size)
-                                       i_size_write(file->f_path.dentry->d_inode,
+       if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) {
+               struct inode *inode = file->f_path.dentry->d_inode;
+/* Do not update local mtime - server will set its actual value on write               
+ *             inode->i_ctime = inode->i_mtime = 
+ *                     current_fs_time(inode->i_sb);*/
+               if (total_written > 0) {
+                       spin_lock(&inode->i_lock);
+                       if (*poffset > file->f_path.dentry->d_inode->i_size)
+                               i_size_write(file->f_path.dentry->d_inode,
                                        *poffset);
-                       }
-                       mark_inode_dirty_sync(file->f_path.dentry->d_inode);
+                       spin_unlock(&inode->i_lock);
                }
+               mark_inode_dirty_sync(file->f_path.dentry->d_inode);    
        }
        FreeXid(xid);
        return total_written;
@@ -1012,18 +1013,18 @@ static ssize_t cifs_write(struct file *file, const char *write_data,
        cifs_stats_bytes_written(pTcon, total_written);
 
        /* since the write may have blocked check these pointers again */
-       if (file->f_path.dentry) {
-               if (file->f_path.dentry->d_inode) {
+       if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) {
 /*BB We could make this contingent on superblock ATIME flag too */
-/*                     file->f_path.dentry->d_inode->i_ctime =
-                       file->f_path.dentry->d_inode->i_mtime = CURRENT_TIME;*/
-                       if (total_written > 0) {
-                               if (*poffset > file->f_path.dentry->d_inode->i_size)
-                                       i_size_write(file->f_path.dentry->d_inode,
-                                                    *poffset);
-                       }
-                       mark_inode_dirty_sync(file->f_path.dentry->d_inode);
+/*             file->f_path.dentry->d_inode->i_ctime =
+               file->f_path.dentry->d_inode->i_mtime = CURRENT_TIME;*/
+               if (total_written > 0) {
+                       spin_lock(&file->f_path.dentry->d_inode->i_lock);
+                       if (*poffset > file->f_path.dentry->d_inode->i_size)
+                               i_size_write(file->f_path.dentry->d_inode,
+                                            *poffset);
+                       spin_unlock(&file->f_path.dentry->d_inode->i_lock);
                }
+               mark_inode_dirty_sync(file->f_path.dentry->d_inode);
        }
        FreeXid(xid);
        return total_written;
@@ -1400,6 +1401,7 @@ static int cifs_commit_write(struct file *file, struct page *page,
        xid = GetXid();
        cFYI(1, ("commit write for page %p up to position %lld for %d", 
                 page, position, to));
+       spin_lock(&inode->i_lock);
        if (position > inode->i_size) {
                i_size_write(inode, position);
                /* if (file->private_data == NULL) {
@@ -1429,6 +1431,7 @@ static int cifs_commit_write(struct file *file, struct page *page,
                        cFYI(1, (" SetEOF (commit write) rc = %d", rc));
                } */
        }
+       spin_unlock(&inode->i_lock);
        if (!PageUptodate(page)) {
                position =  ((loff_t)page->index << PAGE_CACHE_SHIFT) + offset;
                /* can not rely on (or let) writepage write this data */

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 37c6ce87416b3eb540f05e30859e6b262c080dd8..24df13a256e50e335895e2c6e0323556e8a75e92 100644 (file)
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -143,10 +143,10 @@ int cifs_get_inode_info_unix(struct inode **pinode,
                inode->i_gid = le64_to_cpu(findData.Gid);
                inode->i_nlink = le64_to_cpu(findData.Nlinks);
 
+               spin_lock(&inode->i_lock);
                if (is_size_safe_to_change(cifsInfo, end_of_file)) {
                /* can not safely change the file size here if the
                   client is writing to it due to potential races */
-
                        i_size_write(inode, end_of_file);
 
                /* blksize needs to be multiple of two. So safer to default to
@@ -162,6 +162,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
                /* for this calculation */
                        inode->i_blocks = (512 - 1 + num_of_bytes) >> 9;
                }
+               spin_unlock(&inode->i_lock);
 
                if (num_of_bytes < end_of_file)
                        cFYI(1, ("allocation size less than end of file"));
@@ -496,6 +497,8 @@ int cifs_get_inode_info(struct inode **pinode,
                /* BB add code here -
                   validate if device or weird share or device type? */
                }
+               
+               spin_lock(&inode->i_lock);
                if (is_size_safe_to_change(cifsInfo, le64_to_cpu(pfindData->EndOfFile))) {
                        /* can not safely shrink the file size here if the
                           client is writing to it due to potential races */
@@ -506,6 +509,7 @@ int cifs_get_inode_info(struct inode **pinode,
                        inode->i_blocks = (512 - 1 + le64_to_cpu(
                                           pfindData->AllocationSize)) >> 9;
                }
+               spin_unlock(&inode->i_lock);
 
                inode->i_nlink = le32_to_cpu(pfindData->NumberOfLinks);
 
@@ -834,8 +838,10 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry)
 
        if (!rc) {
                drop_nlink(inode);
+               spin_lock(&direntry->d_inode->i_lock);
                i_size_write(direntry->d_inode,0);
                clear_nlink(direntry->d_inode);
+               spin_unlock(&direntry->d_inode->i_lock);
        }
 
        cifsInode = CIFS_I(direntry->d_inode);
@@ -1128,6 +1134,46 @@ static int cifs_truncate_page(struct address_space *mapping, loff_t from)
        return rc;
 }
 
+int cifs_vmtruncate(struct inode * inode, loff_t offset)
+{
+       struct address_space *mapping = inode->i_mapping;
+       unsigned long limit;
+
+       if (inode->i_size < offset)
+               goto do_expand;
+       /*
+        * truncation of in-use swapfiles is disallowed - it would cause
+        * subsequent swapout to scribble on the now-freed blocks.
+        */
+       if (IS_SWAPFILE(inode))
+               goto out_busy;
+       spin_lock(&inode->i_lock);              
+       i_size_write(inode, offset);
+       spin_unlock(&inode->i_lock);
+       unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
+       truncate_inode_pages(mapping, offset);
+       goto out_truncate;
+
+do_expand:
+       limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
+       if (limit != RLIM_INFINITY && offset > limit)
+               goto out_sig;
+       if (offset > inode->i_sb->s_maxbytes)
+               goto out_big;
+       i_size_write(inode, offset);
+
+out_truncate:
+       if (inode->i_op && inode->i_op->truncate)
+               inode->i_op->truncate(inode);
+       return 0;
+out_sig:
+       send_sig(SIGXFSZ, current, 0);
+out_big:
+       return -EFBIG;
+out_busy:
+       return -ETXTBSY;
+}
+
 int cifs_setattr(struct dentry *direntry, struct iattr *attrs)
 {
        int xid;
@@ -1244,7 +1290,7 @@ int cifs_setattr(struct dentry *direntry, struct iattr *attrs)
                   */
 
                if (rc == 0) {
-                       rc = vmtruncate(direntry->d_inode, attrs->ia_size);
+                       rc = cifs_vmtruncate(direntry->d_inode, attrs->ia_size);
                        cifs_truncate_page(direntry->d_inode->i_mapping,
                                           direntry->d_inode->i_size);
                } else 

diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index c444798f0740f91da21052def00e2a9852c383a8..44cfb528797d73548fc8df8ac12eeb4a27063653 100644 (file)
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -3,7 +3,7 @@
  *
  *   Directory search handling
  * 
- *   Copyright (C) International Business Machines  Corp., 2004, 2005
+ *   Copyright (C) International Business Machines  Corp., 2004, 2007
  *   Author(s): Steve French (sfrench@us.ibm.com)
  *
  *   This library is free software; you can redistribute it and/or modify
@@ -226,6 +226,7 @@ static void fill_in_inode(struct inode *tmp_inode, int new_buf_type,
                atomic_set(&cifsInfo->inUse, 1);
        }
 
+       spin_lock(&tmp_inode->i_lock);
        if (is_size_safe_to_change(cifsInfo, end_of_file)) {
                /* can not safely change the file size here if the 
                client is writing to it due to potential races */
@@ -235,6 +236,7 @@ static void fill_in_inode(struct inode *tmp_inode, int new_buf_type,
        /* for this calculation, even though the reported blocksize is larger */
                tmp_inode->i_blocks = (512 - 1 + allocation_size) >> 9;
        }
+       spin_unlock(&tmp_inode->i_lock);
 
        if (allocation_size < end_of_file)
                cFYI(1, ("May be sparse file, allocation less than file size"));
@@ -355,6 +357,7 @@ static void unix_fill_in_inode(struct inode *tmp_inode,
        tmp_inode->i_gid = le64_to_cpu(pfindData->Gid);
        tmp_inode->i_nlink = le64_to_cpu(pfindData->Nlinks);
 
+       spin_lock(&tmp_inode->i_lock);
        if (is_size_safe_to_change(cifsInfo, end_of_file)) {
                /* can not safely change the file size here if the 
                client is writing to it due to potential races */
@@ -364,6 +367,7 @@ static void unix_fill_in_inode(struct inode *tmp_inode,
        /* for this calculation, not the real blocksize */
                tmp_inode->i_blocks = (512 - 1 + num_of_bytes) >> 9;
        }
+       spin_unlock(&tmp_inode->i_lock);
 
        if (S_ISREG(tmp_inode->i_mode)) {
                cFYI(1, ("File inode"));