2012/2/21 Jeff Layton <jlayton@xxxxxxxxx>:
> On Fri, 17 Feb 2012 16:13:30 +0300
> Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
>
>> Currently we do inc/drop_nlink for a parent directory for every
>> mkdir/rmdir calls. That's wrong when Unix extensions are disabled
>> because in this case a server doesn't follow the same semantic and
>> returns the old value on the next QueryInfo request. As the result,
>> we update our value with the server one and then decrement it on
>> every rmdir call - go to negative nlink values.
>>
>> Fix this by removing inc/drop_nlink for the parent directory from
>> mkdir/rmdir, setting it for a revalidation and ignoring NumberOfLinks
>> for directories when Unix extensions are disabled.
>>
>> Signed-off-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
>> ---
>> fs/cifs/inode.c | 28 +++++++++++++++++++---------
>> 1 files changed, 19 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
>> index a5f54b7..745da3d 100644
>> --- a/fs/cifs/inode.c
>> +++ b/fs/cifs/inode.c
>> @@ -534,6 +534,11 @@ cifs_all_info_to_fattr(struct cifs_fattr *fattr, FILE_ALL_INFO *info,
>> if (fattr->cf_cifsattrs & ATTR_DIRECTORY) {
>> fattr->cf_mode = S_IFDIR | cifs_sb->mnt_dir_mode;
>> fattr->cf_dtype = DT_DIR;
>> + /*
>> + * Server can return wrong NumberOfLinks value for directories
>> + * when Unix extensions are disabled - fake it.
>> + */
>> + fattr->cf_nlink = 2;
>> } else {
>> fattr->cf_mode = S_IFREG | cifs_sb->mnt_file_mode;
>> fattr->cf_dtype = DT_REG;
>> @@ -541,9 +546,9 @@ cifs_all_info_to_fattr(struct cifs_fattr *fattr, FILE_ALL_INFO *info,
>> /* clear write bits if ATTR_READONLY is set */
>> if (fattr->cf_cifsattrs & ATTR_READONLY)
>> fattr->cf_mode &= ~(S_IWUGO);
>> - }
>>
>> - fattr->cf_nlink = le32_to_cpu(info->NumberOfLinks);
>> + fattr->cf_nlink = le32_to_cpu(info->NumberOfLinks);
>> + }
>>
>> fattr->cf_uid = cifs_sb->mnt_uid;
>> fattr->cf_gid = cifs_sb->mnt_gid;
>> @@ -1322,7 +1327,6 @@ int cifs_mkdir(struct inode *inode, struct dentry *direntry, umode_t mode)
>> }
>> /*BB check (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID ) to see if need
>> to set uid/gid */
>> - inc_nlink(inode);
>>
>> cifs_unix_basic_to_fattr(&fattr, pInfo, cifs_sb);
>> cifs_fill_uniqueid(inode->i_sb, &fattr);
>> @@ -1355,7 +1359,6 @@ mkdir_retry_old:
>> d_drop(direntry);
>> } else {
>> mkdir_get_info:
>> - inc_nlink(inode);
>> if (pTcon->unix_ext)
>> rc = cifs_get_inode_info_unix(&newinode, full_path,
>> inode->i_sb, xid);
>> @@ -1436,6 +1439,11 @@ mkdir_get_info:
>> }
>> }
>> mkdir_out:
>> + /*
>> + * Force revalidate to get parent dir info when needed since cached
>> + * attributes are invalid now.
>> + */
>> + CIFS_I(inode)->time = 0;
>> kfree(full_path);
>> FreeXid(xid);
>> cifs_put_tlink(tlink);
>> @@ -1475,7 +1483,6 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry)
>> cifs_put_tlink(tlink);
>>
>> if (!rc) {
>> - drop_nlink(inode);
>> spin_lock(&direntry->d_inode->i_lock);
>> i_size_write(direntry->d_inode, 0);
>> clear_nlink(direntry->d_inode);
>> @@ -1483,12 +1490,15 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry)
>> }
>>
>> cifsInode = CIFS_I(direntry->d_inode);
>> - cifsInode->time = 0; /* force revalidate to go get info when
>> - needed */
>> + /* force revalidate to go get info when needed */
>> + cifsInode->time = 0;
>>
>> cifsInode = CIFS_I(inode);
>> - cifsInode->time = 0; /* force revalidate to get parent dir info
>> - since cached search results now invalid */
>> + /*
>> + * Force revalidate to get parent dir info when needed since cached
>> + * attributes are invalid now.
>> + */
>> + cifsInode->time = 0;
>>
>> direntry->d_inode->i_ctime = inode->i_ctime = inode->i_mtime =
>> current_fs_time(inode->i_sb);
>
> Looks good.
>
> Reviewed-by: Jeff Layton <jlayton@xxxxxxxxx>
Steve, what's about this patch? I think it should go to stable as well.
--
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
