fs/super.c set_anon_super calling optimization
Subsystems (5)
A message that is not in the archive (sent off-list or not yet received): CA+kxV1F5VxBGawhkV=02LcgHNWiKF-H_k6N-d1TEYNeFAScezg@mail.gmail.com
PATCH RESEND 1/1 fs/super.c set_anon_super calling optimization
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
---
fs/9p/vfs_super.c +0 -0
@@ -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);
}
/**
fs/btrfs/super.c +0 -0
@@ -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;fs/nfs/super.c +0 -0
@@ -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;fs/sysfs/mount.c +0 -0
@@ -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,
Quoted 8 lines · Abhijit Pawar
Hi,
set_anon_superis 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?
Signature · 1 line
--Carlos
Quoted 9 lines · Carlos Maiolino
Hi,
Quoted 6 lines · Abhijit Pawar
Hi,
set_anon_superis 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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 1 line
Signature · 2 lines
- Abhijit
Al Viro
maintainerQuoted 9 lines · Carlos Maiolino
Hi,
Quoted 6 lines · Abhijit Pawar
Hi,
set_anon_superis 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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 13 lines · Al Viro
Quoted 12 lines · Carlos Maiolino
Hi,
Quoted 8 lines · Abhijit Pawar
Hi,
set_anon_superis 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.
Quoted 7 lines
Quoted 2 lines
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.
Signature · 2 lines
- Abhijit
Al Viro
maintainerQuoted 6 lines · Abhijit Pawar
Quoted 4 lines
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).