Re: [f2fs-dev] [PATCH] f2fs: add F2FS_IOC_SEC_TRIM_FILE ioctl

[Daeho Jeong]

Yes, I saw the implementation in vfs_write().
But if we use mnt_want_write_file() here, it'll call mnt_clone_write()
internally if the file is already open in write mode.
Don't you think the below thing is needed? We can increase the counter
each of them, open and ioctl, like other filesystems such as ext4.

int mnt_clone_write(struct vfsmount *mnt)
{
        /* superblock may be r/o */
        if (__mnt_is_readonly(mnt))
                return -EROFS;
        preempt_disable();
        mnt_inc_writers(real_mount(mnt));
        preempt_enable();
        return 0;
}

2020ë 6ì 11ì (ë) ìì 9:00, Eric Biggers <ebiggers@xxxxxxxxxx>ëì ìì:
>
> 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