linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: Sahil Kang <sahil.kang@asilaycomputing.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 0/1] btrfs: reuse existing pointers from btrfs_ioctl
Date: Thu, 6 Jan 2022 16:11:21 +0100	[thread overview]
Message-ID: <20220106151121.GE14046@twin.jikos.cz> (raw)
In-Reply-To: <74fcaf5f-13f9-dfa0-0576-8a6da3cc8060@gmx.com>

On Wed, Jan 05, 2022 at 05:30:20PM +0800, Qu Wenruo wrote:
> 
> 
> 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.

Agreed.

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

I've noticed there are some calls to file_mnt_user_ns that use the 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)

The main switch function btrfs_ioctl extracts all the pointers so I'd
suggest to pass only the one that the ioctl callbacks need, ie. not an
inode if it's just for the root pointer.

      reply	other threads:[~2022-01-06 15:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-05  8:30 [PATCH 0/1] btrfs: reuse existing pointers from btrfs_ioctl Sahil Kang
2022-01-05  8:30 ` [PATCH 1/1] " Sahil Kang
2022-01-05  9:53   ` Qu Wenruo
2022-01-06 15:03     ` David Sterba
2022-01-16  2:48       ` [PATCH 0/1] btrfs: reuse existing inode " Sahil Kang
2022-01-16  2:48         ` [PATCH 1/1] " Sahil Kang
2022-01-17 13:39           ` David Sterba
2022-01-06 15:27   ` [PATCH 1/1] btrfs: reuse existing pointers " David Sterba
2022-01-05  9:30 ` [PATCH 0/1] " Qu Wenruo
2022-01-06 15:11   ` David Sterba [this message]

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=20220106151121.GE14046@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=quwenruo.btrfs@gmx.com \
    --cc=sahil.kang@asilaycomputing.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 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).