* [RESEND PATCH] fs/super.c set_anon_super calling optimization [not found] <CA+kxV1F5VxBGawhkV=02LcgHNWiKF-H_k6N-d1TEYNeFAScezg@mail.gmail.com> @ 2012-10-25 11:38 ` Abhijit Pawar 2012-10-26 13:14 ` Carlos Maiolino 0 siblings, 1 reply; 6+ messages in thread From: Abhijit Pawar @ 2012-10-25 11:38 UTC (permalink / raw) To: linux-fsdevel; +Cc: linux-btrfs@vger.kernel.org, linux-kernel, gregkh Hi, set_anon_super is called by many filesystems. Some call directly and some call through the wrapper. Many of them in the wrapper's call to this function are passing the second argument to this function which is not used anywhere. This patch replaces the second variable with NULL. Thanks, Abhijit Pawar Signed-off-by: Abhijit Pawar <abhi.c.pawar@gmail.com> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org> CC: linux-kernel@vger.kernel.org CC: linux-btrfs@vger.kernel.org --- diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c index 137d503..132db90 100644 --- a/fs/9p/vfs_super.c +++ b/fs/9p/vfs_super.c @@ -61,7 +61,7 @@ static const struct super_operations v9fs_super_ops, v9fs_super_ops_dotl; static int v9fs_set_super(struct super_block *s, void *data) { s->s_fs_info = data; - return set_anon_super(s, data); + return set_anon_super(s, NULL); } /** diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 915ac14..c9994a3 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -954,7 +954,7 @@ static int btrfs_test_super(struct super_block *s, void *data) static int btrfs_set_super(struct super_block *s, void *data) { - int err = set_anon_super(s, data); + int err = set_anon_super(s, NULL); if (!err) s->s_fs_info = data; return err; diff --git a/fs/nfs/super.c b/fs/nfs/super.c index e831bce..486bf7a 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -2260,7 +2260,7 @@ static int nfs_set_super(struct super_block *s, void *data) s->s_flags = sb_mntdata->mntflags; s->s_fs_info = server; s->s_d_op = server->nfs_client->rpc_ops->dentry_ops; - ret = set_anon_super(s, server); + ret = set_anon_super(s, NULL); if (ret == 0) server->s_dev = s->s_dev; return ret; diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c index 71eb7e2..56d0059 100644 --- a/fs/sysfs/mount.c +++ b/fs/sysfs/mount.c @@ -89,7 +89,7 @@ static int sysfs_test_super(struct super_block *sb, void *data) static int sysfs_set_super(struct super_block *sb, void *data) { int error; - error = set_anon_super(sb, data); + error = set_anon_super(sb, NULL); if (!error) sb->s_fs_info = data; return error; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] fs/super.c set_anon_super calling optimization 2012-10-25 11:38 ` [RESEND PATCH] fs/super.c set_anon_super calling optimization Abhijit Pawar @ 2012-10-26 13:14 ` Carlos Maiolino 2012-10-26 13:40 ` Abhijit Pawar 2012-11-30 4:05 ` Al Viro 0 siblings, 2 replies; 6+ messages in thread From: Carlos Maiolino @ 2012-10-26 13:14 UTC (permalink / raw) To: Abhijit Pawar Cc: linux-fsdevel, linux-btrfs@vger.kernel.org, linux-kernel, gregkh Hi, On Thu, Oct 25, 2012 at 05:08:19PM +0530, Abhijit Pawar wrote: > Hi, > set_anon_super is called by many filesystems. Some call directly and > some call through the wrapper. Many of them in the wrapper's call to > this function are passing the second argument to this function which > is not used anywhere. > > This patch replaces the second variable with NULL. > If the variable isn't used anymore, why don't just get rid of it, instead of call the function passing a NULL pointer on it? -- --Carlos ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] fs/super.c set_anon_super calling optimization 2012-10-26 13:14 ` Carlos Maiolino @ 2012-10-26 13:40 ` Abhijit Pawar 2012-11-30 4:05 ` Al Viro 1 sibling, 0 replies; 6+ messages in thread From: Abhijit Pawar @ 2012-10-26 13:40 UTC (permalink / raw) To: linux-fsdevel, linux-btrfs@vger.kernel.org, linux-kernel, gregkh On 10/26/2012 06:44 PM, Carlos Maiolino wrote: > Hi, > > On Thu, Oct 25, 2012 at 05:08:19PM +0530, Abhijit Pawar wrote: >> Hi, >> set_anon_super is called by many filesystems. Some call directly and >> some call through the wrapper. Many of them in the wrapper's call to >> this function are passing the second argument to this function which >> is not used anywhere. >> >> This patch replaces the second variable with NULL. >> > > If the variable isn't used anymore, why don't just get rid of it, instead of > call the function passing a NULL pointer on it? At the moment its a callback function with two params so that filesystems are free to override it while mounting. This is to support filesystem specific information at mount time. nfs uses it to get its server specific information. btrfs uses it to populate its filesystem information. So the signature can not be changed without affecting these filesystems. sysfs, ceph, 9p will also be affected if we are to change the signature. > -- - Abhijit ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] fs/super.c set_anon_super calling optimization 2012-10-26 13:14 ` Carlos Maiolino 2012-10-26 13:40 ` Abhijit Pawar @ 2012-11-30 4:05 ` Al Viro 2012-11-30 5:40 ` Abhijit Pawar 1 sibling, 1 reply; 6+ messages in thread From: Al Viro @ 2012-11-30 4:05 UTC (permalink / raw) To: Abhijit Pawar, linux-fsdevel, linux-btrfs@vger.kernel.org, linux-kernel, gregkh On Fri, Oct 26, 2012 at 11:14:41AM -0200, Carlos Maiolino wrote: > Hi, > > On Thu, Oct 25, 2012 at 05:08:19PM +0530, Abhijit Pawar wrote: > > Hi, > > set_anon_super is called by many filesystems. Some call directly and > > some call through the wrapper. Many of them in the wrapper's call to > > this function are passing the second argument to this function which > > is not used anywhere. > > > > This patch replaces the second variable with NULL. > > > > If the variable isn't used anymore, why don't just get rid of it, instead of > call the function passing a NULL pointer on it? Because we want it to be a valid sget() callback. I doubt that this optimization is worth doing, though - might even micro-pessimize the things on architectures where all arguments are passed in registers. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] fs/super.c set_anon_super calling optimization 2012-11-30 4:05 ` Al Viro @ 2012-11-30 5:40 ` Abhijit Pawar 2012-11-30 6:04 ` Al Viro 0 siblings, 1 reply; 6+ messages in thread From: Abhijit Pawar @ 2012-11-30 5:40 UTC (permalink / raw) To: Al Viro Cc: Abhijit Pawar, linux-fsdevel, linux-btrfs@vger.kernel.org, linux-kernel, gregkh On 11/30/2012 09:35 AM, Al Viro wrote: > On Fri, Oct 26, 2012 at 11:14:41AM -0200, Carlos Maiolino wrote: >> Hi, >> >> On Thu, Oct 25, 2012 at 05:08:19PM +0530, Abhijit Pawar wrote: >>> Hi, >>> set_anon_super is called by many filesystems. Some call directly and >>> some call through the wrapper. Many of them in the wrapper's call to >>> this function are passing the second argument to this function which >>> is not used anywhere. >>> >>> This patch replaces the second variable with NULL. >>> >> >> If the variable isn't used anymore, why don't just get rid of it, instead of >> call the function passing a NULL pointer on it? > > Because we want it to be a valid sget() callback. I doubt that this > optimization is worth doing, though - might even micro-pessimize the things > on architectures where all arguments are passed in registers. > Al, Yes. it will be helpful in registers case. -- - Abhijit ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] fs/super.c set_anon_super calling optimization 2012-11-30 5:40 ` Abhijit Pawar @ 2012-11-30 6:04 ` Al Viro 0 siblings, 0 replies; 6+ messages in thread From: Al Viro @ 2012-11-30 6:04 UTC (permalink / raw) To: Abhijit Pawar Cc: linux-fsdevel, linux-btrfs@vger.kernel.org, linux-kernel, gregkh On Fri, Nov 30, 2012 at 11:10:02AM +0530, Abhijit Pawar wrote: > > Because we want it to be a valid sget() callback. I doubt that this > > optimization is worth doing, though - might even micro-pessimize the things > > on architectures where all arguments are passed in registers. > > > Al, > Yes. it will be helpful in registers case. How so? Consider something like static int btrfs_set_super(struct super_block *s, void *data) { int err = set_anon_super(s, data); if (!err) s->s_fs_info = data; return err; } Compile it e.g. for alpha. Or powerpc. Or amd64, for that matter. With and without your change. And compare the resulting assembler. Hell, if the arguments are passed in register, without your patch we have the args for set_anon_super() all set just as we enter btrfs_set_super(). With your patch the second one needs to be zeroed out... In any case, that's microoptimization in the best case and on quite a few architectures it's a pessimization (granted, an equally minor one). ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-11-30 6:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CA+kxV1F5VxBGawhkV=02LcgHNWiKF-H_k6N-d1TEYNeFAScezg@mail.gmail.com>
2012-10-25 11:38 ` [RESEND PATCH] fs/super.c set_anon_super calling optimization Abhijit Pawar
2012-10-26 13:14 ` Carlos Maiolino
2012-10-26 13:40 ` Abhijit Pawar
2012-11-30 4:05 ` Al Viro
2012-11-30 5:40 ` Abhijit Pawar
2012-11-30 6:04 ` Al Viro
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).