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 <[email protected]>
CC: Greg Kroah-Hartman <[email protected]>
CC: [email protected]
CC: [email protected]
---
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;
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
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
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.
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
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).