Re: [RFC PATCH] cifs: remove superfluous inode_lock in cifs_{strict_}fsync

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



It passed automated testing (the most recent test bucket run was
http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/4/builds/141
but it also passed the other test group
http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/185)


On Mon, Apr 8, 2019 at 6:09 PM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>
> Acked-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
>
> --
> Best regards,
> Pavel Shilovsky
>
> пн, 8 апр. 2019 г. в 09:06, Steve French <smfrench@xxxxxxxxx>:
> >
> > Restarted the automated tests after updating for-next and added this patch ontop
> >
> > http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/185
> >
> > On Mon, Apr 8, 2019 at 10:29 AM Steve French <smfrench@xxxxxxxxx> wrote:
> > >
> > > Kicked off automated tests with this patch included - see
> > > http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/184
> > >  (I will kick off other test buckets later)
> > >
> > > On Mon, Apr 8, 2019 at 9:59 AM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > > >
> > > > Originally, filemap_write_and_wait took the i_mutex internally, but
> > > > commit 02c24a82187d pushed the mutex acquisition into the individual
> > > > fsync routines, leaving it up to the subsystem maintainers to remove
> > > > it if it wasn't needed.
> > > >
> > > > For cifs, I see no reason to take the inode_lock here. All of the
> > > > operations inside that lock are protected in other ways.
> > > >
> > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > > > ---
> > > >  fs/cifs/file.c | 5 -----
> > > >  1 file changed, 5 deletions(-)
> > > >
> > > > Steve, would you or someone else be able to test this?
> > > >
> > > > I'm proposing a similar patch for kcephfs. I don't have an appropriate
> > > > test rig at the moment, and I was hoping someone more involved with
> > > > cifs.ko development these days may be able to do so.
> > > >
> > > > Thanks,
> > > > Jeff
> > > >
> > > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> > > > index 89006e044973..c4b84fd423c3 100644
> > > > --- a/fs/cifs/file.c
> > > > +++ b/fs/cifs/file.c
> > > > @@ -2424,7 +2424,6 @@ int cifs_strict_fsync(struct file *file, loff_t start, loff_t end,
> > > >         rc = file_write_and_wait_range(file, start, end);
> > > >         if (rc)
> > > >                 return rc;
> > > > -       inode_lock(inode);
> > > >
> > > >         xid = get_xid();
> > > >
> > > > @@ -2449,7 +2448,6 @@ int cifs_strict_fsync(struct file *file, loff_t start, loff_t end,
> > > >         }
> > > >
> > > >         free_xid(xid);
> > > > -       inode_unlock(inode);
> > > >         return rc;
> > > >  }
> > > >
> > > > @@ -2461,12 +2459,10 @@ int cifs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> > > >         struct TCP_Server_Info *server;
> > > >         struct cifsFileInfo *smbfile = file->private_data;
> > > >         struct cifs_sb_info *cifs_sb = CIFS_FILE_SB(file);
> > > > -       struct inode *inode = file->f_mapping->host;
> > > >
> > > >         rc = file_write_and_wait_range(file, start, end);
> > > >         if (rc)
> > > >                 return rc;
> > > > -       inode_lock(inode);
> > > >
> > > >         xid = get_xid();
> > > >
> > > > @@ -2483,7 +2479,6 @@ int cifs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> > > >         }
> > > >
> > > >         free_xid(xid);
> > > > -       inode_unlock(inode);
> > > >         return rc;
> > > >  }
> > > >
> > > > --
> > > > 2.20.1
> > > >
> > >
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve



-- 
Thanks,

Steve




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux