Re: [PATCH 0/1] btrfs: reuse existing pointers from btrfs_ioctl

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2022/1/5 16:30, Sahil Kang wrote:
> This is a small cleanup that reuses some of the existing pointers and
> removes the duplicated dereferencing.

Some quick glance indeed shows that we have pretty inconsistent
arguments for a lot of ioctls, mostly switching between file/inode.

So the cleanup looks good to me.

But I also find that, there are ioctl sub-routines that requires file as
it needs to call mnt_want_write_file().

I know I'm asking for too much, but maybe it's a good idea to separate
those ioctl sub-routines into two types:

- Those write ioctls which need @file to do mnt_want_write_file() check
   So they can use the existing @file argument as usual

- Those read-only ioctls which don't need @file at all
   Then pass @inode/@root to those functions.
   (Personally speaking I prefer to pass @inode and let sub-routines to
    handle the root grabbing)

Anyway, your patch looks like a good initial step.

Will give more detailed review on the patch.

Thanks,
Qu

>
> There are other subfunctions that have similar/identical dereferencing
> as the enclosing btrfs_ioctl, but I'm not sure if changing those is
> welcomed since it would require changing their arg count. Right now,
> all of the sub ioctl functions have essentially the same signature.
>
> For example, reusing the pointers for btrfs_ioctl_set_fslabel would
> require passing in *fs_info and *root, in addition to its current
> args.
>
> Sahil Kang (1):
>    btrfs: reuse existing pointers from btrfs_ioctl
>
>   fs/btrfs/ioctl.c | 28 +++++++++++-----------------
>   1 file changed, 11 insertions(+), 17 deletions(-)
>