Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 10 Jun 2020 17:00:37 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Daeho Jeong <daeho43@gmail.com>
Cc:     linux-kernel@vger.kernel.org,
        linux-f2fs-devel@lists.sourceforge.net, kernel-team@android.com,
        Daeho Jeong <daehojeong@google.com>
Subject: Re: [f2fs-dev] [PATCH] f2fs: add F2FS_IOC_SEC_TRIM_FILE ioctl
Message-ID: <20200611000037.GC1339@sol.localdomain>
References: <20200609060137.143501-1-daeho43@gmail.com>
 <20200609165107.GA228564@gmail.com>
 <CACOAw_xEZ+au9yhFerq9amkRO62Dzxj7h71gEc=i16ReYu5xrg@mail.gmail.com>
 <20200610031532.GA6286@sol.localdomain>
 <CACOAw_wErOPC=Kf3UU8nFGhWRy84ZnCeJbsyPhSCcXv51B_XxQ@mail.gmail.com>
 <CACOAw_zka6d06RxFOUTwEV7B6o8A2-_6FvqWh_A1nJ0+7FU9yQ@mail.gmail.com>
 <CACOAw_yc4hxdyxyO+Lb4MArHek1tP4wxCq0tezWOocgqvK+tqg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACOAw_yc4hxdyxyO+Lb4MArHek1tP4wxCq0tezWOocgqvK+tqg@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jun 11, 2020 at 08:53:10AM +0900, Daeho Jeong wrote:
> > > > > Using FMODE_WRITE is more proper for this case, since we're going to
> > > > > modify the data. But I think mnt_want_write_file() is still required
> > > > > to prevent the filesystem from freezing or something else.
> > > >
> > > > Right, the freezing check is actually still necessary.  But getting write access
> > > > to the mount is not necessary.  I think you should use file_start_write() and
> > > > file_end_write(), like vfs_write() does.
> >
> > I've checked this again.
> >
> > But I think mnt_want_write_file() looks better than the combination of
> > checking FMODE_WRITE and file_start_write(), because
> > mnt_want_write_file() handles all the things we need.
> > It checks FMODE_WRITER, which is set in do_dentry_open() when
> > FMODE_WRITE is already set, and does the stuff that file_start_write()
> > is doing. This is why the other filesystem system calls use it.
> >
> > What do you think?
> 
> Hmm, we still need FMODE_WRITE check.
> But mnt_want_write_file() looks better, because it'll call
> mnt_clone_write() internally, if the file is open for write already.

There's no need to get write access to the mount if you already have a writable
fd.  You just need file_start_write() for the freeze protection.  Again, see
vfs_write().

- Eric