From: Andreas Gruenbacher <agruen@suse.de>
To: Marcelo Tossati <marcelo@conectiva.com.br>
Cc: linux-fsdevel@vger.kernel.org, Andrew Morton <akpm@osdl.org>,
Andrea Arcangeli <andrea@suse.de>,
Dave Kleikamp <shaggy@austin.ibm.com>,
Nathan Scott <nathans@sgi.com>, Chris Mason <mason@suse.com>,
Tim Shimmin <tes@sgi.com>, Jeff Mahoney <jeffm@suse.com>,
Steve Best <sbest@us.ibm.com>
Subject: xattr iops locking improvement
Date: Mon, 7 Jul 2003 19:48:04 +0200 [thread overview]
Message-ID: <200307071948.04703.agruen@suse.de> (raw)
[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]
Hi Marcelo,
Andrew Morton has found that taking i_sem for all xattr iops leads to lock
contention. Particularly annoying is that writers can block operations like
`ls -l' because both need i_sem. We have bene working on a fix for 2.4 and
2.5. The solution is a series of patches that are in Andrew's current tree,
and have been sent to Linus. I will also shortly put them on
http://acl.bestbits.at/.
For vanilla 2.4 the relevant patch is xattr-fine-grained-locking-1. i_sem is
no longer taken in the listxattr and getxattr iops. It is still taken for
setxattr and removexattr. The patch moves the lock taking down to the file
system level.
I have patches ready that remove the need for i_sem in ext2 and ext3, and I'm
sure the xfs, jfs, and reiserfs coders will be quick to follow up with
improved xattr and acl patches as well.
I am attaching xattr-fine-grained-locking-2 and xattr-fine-grained-locking-3
for reference.
Cheers,
Andreas.
------------------------------------------------------------------
Andreas Gruenbacher SuSE Linux AG
mailto:agruen@suse.de Deutschherrnstr. 15-19
http://www.suse.de/ D-90429 Nuernberg, Germany
[-- Attachment #2: xattr-fine-grained-locking-1 --]
[-- Type: text/x-diff, Size: 3138 bytes --]
More fine-grained locking for extended attributes
No longer take inode->i_sem for iops->getxattr and iops->listxattr.
Move taking inode->i_sem down to the file system level.
Andreas Gruenbacher <agruen@suse.de>, SuSE Labs
Index: linux-2.4.21/fs/xattr.c
===================================================================
--- linux-2.4.21.orig/fs/xattr.c 2002-11-29 00:53:15.000000000 +0100
+++ linux-2.4.21/fs/xattr.c 2003-06-25 22:03:05.000000000 +0200
@@ -159,11 +159,9 @@ getxattr(struct dentry *d, char *name, v
error = -EOPNOTSUPP;
if (d->d_inode->i_op && d->d_inode->i_op->getxattr) {
- down(&d->d_inode->i_sem);
lock_kernel();
error = d->d_inode->i_op->getxattr(d, kname, kvalue, size);
unlock_kernel();
- up(&d->d_inode->i_sem);
}
if (kvalue && error > 0)
@@ -230,11 +228,9 @@ listxattr(struct dentry *d, char *list,
error = -EOPNOTSUPP;
if (d->d_inode->i_op && d->d_inode->i_op->listxattr) {
- down(&d->d_inode->i_sem);
lock_kernel();
error = d->d_inode->i_op->listxattr(d, klist, size);
unlock_kernel();
- up(&d->d_inode->i_sem);
}
if (klist && error > 0)
Index: linux-2.4.21/fs/jfs/xattr.c
===================================================================
--- linux-2.4.21.orig/fs/jfs/xattr.c 2003-06-25 22:00:29.000000000 +0200
+++ linux-2.4.21/fs/jfs/xattr.c 2003-06-25 22:17:53.000000000 +0200
@@ -964,12 +964,18 @@ 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 inode *inode, char *data,
+ size_t buf_size)
{
- struct inode *inode = dentry->d_inode;
char *buffer;
ssize_t size = 0;
int xattr_size;
@@ -1013,6 +1019,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);
+
+ return err;
+}
+
int jfs_removexattr(struct dentry *dentry, const char *name)
{
return __jfs_setxattr(dentry->d_inode, name, 0, 0, XATTR_REPLACE);
Index: linux-2.4.21/Documentation/filesystems/Locking
===================================================================
--- linux-2.4.21.orig/Documentation/filesystems/Locking 2003-06-25 21:58:40.000000000 +0200
+++ linux-2.4.21/Documentation/filesystems/Locking 2003-06-25 22:03:05.000000000 +0200
@@ -69,8 +69,8 @@ permission: yes no no
getattr: (see below)
revalidate: no (see below)
setxattr: yes yes no
-getxattr: yes yes no
-listxattr: yes yes no
+getxattr: yes no no
+listxattr: yes no no
removexattr: yes yes no
Additionally, ->rmdir() has i_zombie on victim and so does ->rename()
in case when target exists and is a directory.
[-- Attachment #3: xattr-fine-grained-locking-2 --]
[-- Type: text/x-diff, Size: 2066 bytes --]
Index: linux-2.4.21/fs/ext2/xattr.c
===================================================================
--- linux-2.4.21.orig/fs/ext2/xattr.c 2003-06-25 15:29:13.000000000 +0200
+++ linux-2.4.21/fs/ext2/xattr.c 2003-06-25 15:59:05.000000000 +0200
@@ -290,11 +290,17 @@ 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;
}
/*
@@ -306,7 +312,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;
}
/*
Index: linux-2.4.21/fs/ext3/xattr.c
===================================================================
--- linux-2.4.21.orig/fs/ext3/xattr.c 2003-06-25 15:29:13.000000000 +0200
+++ linux-2.4.21/fs/ext3/xattr.c 2003-06-25 15:58:50.000000000 +0200
@@ -289,11 +289,17 @@ 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;
}
/*
@@ -305,7 +311,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;
}
/*
[-- Attachment #4: xattr-fine-grained-locking-3 --]
[-- Type: text/x-diff, Size: 2843 bytes --]
Index: linux-2.4.21/fs/reiserfs/xattr.c
===================================================================
--- linux-2.4.21.orig/fs/reiserfs/xattr.c 2003-06-25 15:29:19.000000000 +0200
+++ linux-2.4.21/fs/reiserfs/xattr.c 2003-06-25 16:04:39.000000000 +0200
@@ -1322,7 +1322,9 @@ reiserfs_getxattr (struct dentry *dentry
return -EOPNOTSUPP;
reiserfs_read_lock_xattrs (dentry->d_sb);
+ down(&dentry->d_inode->i_sem);
err = xah->get (dentry->d_inode, name, buffer, size);
+ up(&dentry->d_inode->i_sem);
reiserfs_read_unlock_xattrs (dentry->d_sb);
return err;
}
@@ -1426,14 +1428,9 @@ reiserfs_listxattr_filler (void *buf, co
return 0;
}
-/*
- * Inode operation listxattr()
- *
- * dentry->d_inode->i_sem down
- * BKL held [before 2.5.x]
- */
-ssize_t
-reiserfs_listxattr (struct dentry *dentry, char *buffer, size_t size)
+
+static ssize_t
+__reiserfs_listxattr (struct dentry *dentry, char *buffer, size_t size)
{
struct file *fp;
struct dentry *dir;
@@ -1485,6 +1482,24 @@ out:
return err;
}
+/*
+ * Inode operation listxattr()
+ *
+ * dentry->d_inode->i_sem down
+ * BKL held [before 2.5.x]
+ */
+ssize_t
+reiserfs_listxattr (struct dentry *dentry, char *buffer, size_t size)
+{
+ int error;
+
+ down(&dentry->d_inode->i_sem);
+ error = __reiserfs_listxattr(dentry, buffer, size);
+ up(&dentry->d_inode->i_sem);
+
+ return error;
+}
+
/* This is the implementation for the xattr plugin infrastructure */
static struct reiserfs_xattr_handler *xattr_handlers;
static rwlock_t handler_lock = RW_LOCK_UNLOCKED;
Index: linux-2.4.21/fs/xfs/linux/xfs_iops.c
===================================================================
--- linux-2.4.21.orig/fs/xfs/linux/xfs_iops.c 2003-06-25 15:28:43.000000000 +0200
+++ linux-2.4.21/fs/xfs/linux/xfs_iops.c 2003-06-25 16:13:33.000000000 +0200
@@ -644,7 +644,7 @@ linvfs_setxattr(
}
STATIC ssize_t
-linvfs_getxattr(
+__linvfs_getxattr(
struct dentry *dentry,
const char *name,
void *data,
@@ -699,9 +699,25 @@ 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)
@@ -743,6 +759,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,
next reply other threads:[~2003-07-07 17:33 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-07-07 17:48 Andreas Gruenbacher [this message]
2003-07-07 17:57 ` xattr iops locking improvement Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200307071948.04703.agruen@suse.de \
--to=agruen@suse.de \
--cc=akpm@osdl.org \
--cc=andrea@suse.de \
--cc=jeffm@suse.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=marcelo@conectiva.com.br \
--cc=mason@suse.com \
--cc=nathans@sgi.com \
--cc=sbest@us.ibm.com \
--cc=shaggy@austin.ibm.com \
--cc=tes@sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.