From: Andreas Gruenbacher Andrew Morton found that there is lock contention between extended attribute operations (like reading ACLs, which `ls -l' needs to do) and other operations on the same files. This is due to the fact that all extended attribute syscalls take inode->i_sem before calling into the filesystem code. To fix this problem, this patch no longer takes inode->i_sem in the getxattr and listxattr syscalls, and moves the lock taking code into the file systems. (Another patch improves the locking strategy in ext2 and ext3.) Documentation/filesystems/Locking | 4 ++-- fs/ext2/xattr.c | 15 +++++++++++++-- fs/ext3/xattr.c | 15 +++++++++++++-- fs/jfs/xattr.c | 22 ++++++++++++++++++++-- fs/xattr.c | 4 ---- fs/xfs/linux/xfs_iops.c | 34 ++++++++++++++++++++++++++++++++-- 6 files changed, 80 insertions(+), 14 deletions(-) diff -puN Documentation/filesystems/Locking~xattr-fine-grained-locking-prep Documentation/filesystems/Locking --- 25/Documentation/filesystems/Locking~xattr-fine-grained-locking-prep 2003-07-02 16:37:42.000000000 -0700 +++ 25-akpm/Documentation/filesystems/Locking 2003-07-02 16:37:42.000000000 -0700 @@ -68,8 +68,8 @@ setattr: yes permission: no getattr: no setxattr: yes -getxattr: yes -listxattr: yes +getxattr: no +listxattr: no removexattr: yes Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_sem on victim. diff -puN fs/ext2/xattr.c~xattr-fine-grained-locking-prep fs/ext2/xattr.c --- 25/fs/ext2/xattr.c~xattr-fine-grained-locking-prep 2003-07-02 16:37:42.000000000 -0700 +++ 25-akpm/fs/ext2/xattr.c 2003-07-02 16:37:42.000000000 -0700 @@ -204,11 +204,16 @@ ext2_getxattr(struct dentry *dentry, con { struct ext2_xattr_handler *handler; struct inode *inode = dentry->d_inode; + ssize_t error; handler = ext2_xattr_resolve_name(&name); if (!handler) return -EOPNOTSUPP; - return handler->get(inode, name, buffer, size); + down(&inode->i_sem); + error = handler->get(inode, name, buffer, size); + up(&inode->i_sem); + + return error; } /* @@ -219,7 +224,13 @@ ext2_getxattr(struct dentry *dentry, con ssize_t ext2_listxattr(struct dentry *dentry, char *buffer, size_t size) { - return ext2_xattr_list(dentry->d_inode, buffer, size); + ssize_t error; + + down(&dentry->d_inode->i_sem); + error = ext2_xattr_list(dentry->d_inode, buffer, size); + up(&dentry->d_inode->i_sem); + + return error; } /* diff -puN fs/ext3/xattr.c~xattr-fine-grained-locking-prep fs/ext3/xattr.c --- 25/fs/ext3/xattr.c~xattr-fine-grained-locking-prep 2003-07-02 16:37:42.000000000 -0700 +++ 25-akpm/fs/ext3/xattr.c 2003-07-02 16:37:43.000000000 -0700 @@ -199,11 +199,16 @@ ext3_getxattr(struct dentry *dentry, con { struct ext3_xattr_handler *handler; struct inode *inode = dentry->d_inode; + ssize_t error; handler = ext3_xattr_resolve_name(&name); if (!handler) return -EOPNOTSUPP; - return handler->get(inode, name, buffer, size); + down(&inode->i_sem); + error = handler->get(inode, name, buffer, size); + up(&inode->i_sem); + + return error; } /* @@ -214,7 +219,13 @@ ext3_getxattr(struct dentry *dentry, con ssize_t ext3_listxattr(struct dentry *dentry, char *buffer, size_t size) { - return ext3_xattr_list(dentry->d_inode, buffer, size); + ssize_t error; + + down(&dentry->d_inode->i_sem); + error = ext3_xattr_list(dentry->d_inode, buffer, size); + up(&dentry->d_inode->i_sem); + + return error; } /* diff -puN fs/jfs/xattr.c~xattr-fine-grained-locking-prep fs/jfs/xattr.c --- 25/fs/jfs/xattr.c~xattr-fine-grained-locking-prep 2003-07-02 16:37:42.000000000 -0700 +++ 25-akpm/fs/jfs/xattr.c 2003-07-02 16:37:43.000000000 -0700 @@ -964,10 +964,17 @@ ssize_t __jfs_getxattr(struct inode *ino ssize_t jfs_getxattr(struct dentry *dentry, const char *name, void *data, size_t buf_size) { - return __jfs_getxattr(dentry->d_inode, name, data, buf_size); + int err; + + down(&dentry->d_inode->i_sem); + err = __jfs_getxattr(dentry->d_inode, name, data, buf_size); + up(&dentry->d_inode->i_sem); + + return err; } -ssize_t jfs_listxattr(struct dentry * dentry, char *data, size_t buf_size) +static ssize_t __jfs_listxattr(struct dentry * dentry, char *data, + size_t buf_size) { struct inode *inode = dentry->d_inode; char *buffer; @@ -1013,6 +1020,17 @@ ssize_t jfs_listxattr(struct dentry * de return size; } +ssize_t jfs_listxattr(struct dentry * dentry, char *data, size_t buf_size) +{ + int err; + + down(&dentry->d_inode->i_sem); + err = __jfs_listxattr(dentry->d_inode, data, buf_size); + up(&dentry->d_inode->i_sem); + + rerturn err; +} + int jfs_removexattr(struct dentry *dentry, const char *name) { return __jfs_setxattr(dentry->d_inode, name, 0, 0, XATTR_REPLACE); diff -puN fs/xattr.c~xattr-fine-grained-locking-prep fs/xattr.c --- 25/fs/xattr.c~xattr-fine-grained-locking-prep 2003-07-02 16:37:42.000000000 -0700 +++ 25-akpm/fs/xattr.c 2003-07-02 16:37:43.000000000 -0700 @@ -160,9 +160,7 @@ getxattr(struct dentry *d, char *name, v error = security_inode_getxattr(d, kname); if (error) goto out; - down(&d->d_inode->i_sem); error = d->d_inode->i_op->getxattr(d, kname, kvalue, size); - up(&d->d_inode->i_sem); } if (kvalue && error > 0) @@ -233,9 +231,7 @@ listxattr(struct dentry *d, char *list, error = security_inode_listxattr(d); if (error) goto out; - down(&d->d_inode->i_sem); error = d->d_inode->i_op->listxattr(d, klist, size); - up(&d->d_inode->i_sem); } if (klist && error > 0) diff -puN fs/xfs/linux/xfs_iops.c~xattr-fine-grained-locking-prep fs/xfs/linux/xfs_iops.c --- 25/fs/xfs/linux/xfs_iops.c~xattr-fine-grained-locking-prep 2003-07-02 16:37:42.000000000 -0700 +++ 25-akpm/fs/xfs/linux/xfs_iops.c 2003-07-02 16:37:43.000000000 -0700 @@ -639,7 +639,7 @@ linvfs_setxattr( } STATIC ssize_t -linvfs_getxattr( +__linvfs_getxattr( struct dentry *dentry, const char *name, void *data, @@ -694,9 +694,24 @@ linvfs_getxattr( return -EOPNOTSUPP; } +STATIC ssize_t +linvfs_getxattr( + struct dentry *dentry, + const char *name, + void *data, + size_t size) +{ + int error; + + down(&dentry->d_inode->i_sem); + error = __linvfs_getxattr(dentry, name, data, size); + up(&dentry->d_inode->i_sem); + + return error; +} STATIC ssize_t -linvfs_listxattr( +__linvfs_listxattr( struct dentry *dentry, char *data, size_t size) @@ -738,6 +753,21 @@ linvfs_listxattr( return result; } +STATIC ssize_t +linvfs_listxattr( + struct dentry *dentry, + char *data, + size_t size) +{ + int error; + + down(&dentry->d_inode->i_sem); + error = __linvfs_listxattr(dentry, data, size); + up(&dentry->d_inode->i_sem); + + return error; +} + STATIC int linvfs_removexattr( struct dentry *dentry, _