From: Steve French Date: Mon, 26 Feb 2007 16:46:11 +0000 (+0000) Subject: [CIFS] Fix locking problem around some cifs uses of i_size write X-Git-Tag: v2.6.21-rc3~21^2~6 X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3677db10a635a39f63ea509f8f0056d95589ff90;p=linux-2.6 [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 CC: Shaggy Signed-off-by: Steve French --- diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES index 5fe13593b5..e08a147c09 100644 --- 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 a1265c9bfe..c07ff8317a 100644 --- 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 37c6ce8741..24df13a256 100644 --- 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 c444798f07..44cfb52879 100644 --- 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"));