| Message ID | 1400842221-12168-1-git-send-email-jlayton@poochiereds.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 23 May 2014 06:50:21 -0400 Jeff Layton <jlayton@poochiereds.net> wrote: > Before satisfying a read with cache=loose, we should always check > that the pagecache is valid before allowing a read to be satisfied > out of it. > > Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Signed-off-by: Jeff Layton <jlayton@poochiereds.net> > --- > fs/cifs/cifsfs.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c > index 2c90d07c0b3a..888398067420 100644 > --- a/fs/cifs/cifsfs.c > +++ b/fs/cifs/cifsfs.c > @@ -725,6 +725,19 @@ out_nls: > goto out; > } > > +static ssize_t > +cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter) > +{ > + ssize_t rc; > + struct inode *inode = file_inode(iocb->ki_filp); > + > + rc = cifs_revalidate_mapping(inode); > + if (rc) > + return rc; > + > + return generic_file_read_iter(iocb, iter); > +} > + > static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) > { > struct inode *inode = file_inode(iocb->ki_filp); > @@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = { > const struct file_operations cifs_file_ops = { > .read = new_sync_read, > .write = new_sync_write, > - .read_iter = generic_file_read_iter, > + .read_iter = cifs_loose_read_iter, > .write_iter = cifs_file_write_iter, > .open = cifs_open, > .release = cifs_close, > @@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = { > const struct file_operations cifs_file_nobrl_ops = { > .read = new_sync_read, > .write = new_sync_write, > - .read_iter = generic_file_read_iter, > + .read_iter = cifs_loose_read_iter, > .write_iter = cifs_file_write_iter, > .open = cifs_open, > .release = cifs_close, Steve, This patch is a replacement for the last patch in the 4 patch series for handling reads when cache=loose. The reason for the respin is that aio_read has been replaced by read_iter in linux-next, so this is what we'll want for v3.16 (once Al's read_iter patches are merged). Thanks,
Yes - makes sense. I am rebuilding for-next branch of cifs-2.6.git now. I plan to put your patch on the tip of the branch - I may create two branches, one with old and one with new version of the patch since when I am testing latest cifs patches (and also the proposed SMB3 Posix extensions) don't have Al's series. On Fri, May 23, 2014 at 9:44 AM, Jeff Layton <jlayton@poochiereds.net> wrote: > On Fri, 23 May 2014 06:50:21 -0400 > Jeff Layton <jlayton@poochiereds.net> wrote: > >> Before satisfying a read with cache=loose, we should always check >> that the pagecache is valid before allowing a read to be satisfied >> out of it. >> >> Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >> Signed-off-by: Jeff Layton <jlayton@poochiereds.net> >> --- >> fs/cifs/cifsfs.c | 17 +++++++++++++++-- >> 1 file changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c >> index 2c90d07c0b3a..888398067420 100644 >> --- a/fs/cifs/cifsfs.c >> +++ b/fs/cifs/cifsfs.c >> @@ -725,6 +725,19 @@ out_nls: >> goto out; >> } >> >> +static ssize_t >> +cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter) >> +{ >> + ssize_t rc; >> + struct inode *inode = file_inode(iocb->ki_filp); >> + >> + rc = cifs_revalidate_mapping(inode); >> + if (rc) >> + return rc; >> + >> + return generic_file_read_iter(iocb, iter); >> +} >> + >> static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) >> { >> struct inode *inode = file_inode(iocb->ki_filp); >> @@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = { >> const struct file_operations cifs_file_ops = { >> .read = new_sync_read, >> .write = new_sync_write, >> - .read_iter = generic_file_read_iter, >> + .read_iter = cifs_loose_read_iter, >> .write_iter = cifs_file_write_iter, >> .open = cifs_open, >> .release = cifs_close, >> @@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = { >> const struct file_operations cifs_file_nobrl_ops = { >> .read = new_sync_read, >> .write = new_sync_write, >> - .read_iter = generic_file_read_iter, >> + .read_iter = cifs_loose_read_iter, >> .write_iter = cifs_file_write_iter, >> .open = cifs_open, >> .release = cifs_close, > > Steve, > > This patch is a replacement for the last patch in the 4 patch series > for handling reads when cache=loose. The reason for the respin is that > aio_read has been replaced by read_iter in linux-next, so this is what > we'll want for v3.16 (once Al's read_iter patches are merged). > > Thanks, > -- > Jeff Layton <jlayton@poochiereds.net>
I pushed the other cifs patches to cifs-2.6.git for-next (and created a for-next-without-aio-iter branch that also includes the same set of cifs patch and also includes the older version of your revalidate patch that builds on current kernels) but your revalidate read_iter patch is not going to merge to for-next without me picking up at a minimum the patch that adds read_iter and write_iter to fs/cifs. Suggestions? On Fri, May 23, 2014 at 9:52 AM, Steve French <smfrench@gmail.com> wrote: > Yes - makes sense. I am rebuilding for-next branch of cifs-2.6.git > now. I plan to put your patch on the tip of the branch - I may create > two branches, one with old and one with new version of the patch since > when I am testing latest cifs patches (and also the proposed SMB3 > Posix extensions) don't have Al's series. > > On Fri, May 23, 2014 at 9:44 AM, Jeff Layton <jlayton@poochiereds.net> wrote: >> On Fri, 23 May 2014 06:50:21 -0400 >> Jeff Layton <jlayton@poochiereds.net> wrote: >> >>> Before satisfying a read with cache=loose, we should always check >>> that the pagecache is valid before allowing a read to be satisfied >>> out of it. >>> >>> Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >>> Signed-off-by: Jeff Layton <jlayton@poochiereds.net> >>> --- >>> fs/cifs/cifsfs.c | 17 +++++++++++++++-- >>> 1 file changed, 15 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c >>> index 2c90d07c0b3a..888398067420 100644 >>> --- a/fs/cifs/cifsfs.c >>> +++ b/fs/cifs/cifsfs.c >>> @@ -725,6 +725,19 @@ out_nls: >>> goto out; >>> } >>> >>> +static ssize_t >>> +cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter) >>> +{ >>> + ssize_t rc; >>> + struct inode *inode = file_inode(iocb->ki_filp); >>> + >>> + rc = cifs_revalidate_mapping(inode); >>> + if (rc) >>> + return rc; >>> + >>> + return generic_file_read_iter(iocb, iter); >>> +} >>> + >>> static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) >>> { >>> struct inode *inode = file_inode(iocb->ki_filp); >>> @@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = { >>> const struct file_operations cifs_file_ops = { >>> .read = new_sync_read, >>> .write = new_sync_write, >>> - .read_iter = generic_file_read_iter, >>> + .read_iter = cifs_loose_read_iter, >>> .write_iter = cifs_file_write_iter, >>> .open = cifs_open, >>> .release = cifs_close, >>> @@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = { >>> const struct file_operations cifs_file_nobrl_ops = { >>> .read = new_sync_read, >>> .write = new_sync_write, >>> - .read_iter = generic_file_read_iter, >>> + .read_iter = cifs_loose_read_iter, >>> .write_iter = cifs_file_write_iter, >>> .open = cifs_open, >>> .release = cifs_close, >> >> Steve, >> >> This patch is a replacement for the last patch in the 4 patch series >> for handling reads when cache=loose. The reason for the respin is that >> aio_read has been replaced by read_iter in linux-next, so this is what >> we'll want for v3.16 (once Al's read_iter patches are merged). >> >> Thanks, >> -- >> Jeff Layton <jlayton@poochiereds.net> > > > > -- > Thanks, > > Steve
On Fri, 23 May 2014 10:33:41 -0500 Steve French <smfrench@gmail.com> wrote: > I pushed the other cifs patches to cifs-2.6.git for-next (and created > a for-next-without-aio-iter branch that also includes the same set of > cifs patch and also includes the older version of your revalidate > patch that builds on current kernels) but your revalidate read_iter > patch is not going to merge to for-next without me picking up at a > minimum the patch that adds read_iter and write_iter to fs/cifs. > Suggestions? > Tough call... I'm not sure how to handle that. One possibility would be to rebase your for-next on top of Al's tree? Sort of icky, but I'm not sure what else can be done... > On Fri, May 23, 2014 at 9:52 AM, Steve French <smfrench@gmail.com> wrote: > > Yes - makes sense. I am rebuilding for-next branch of cifs-2.6.git > > now. I plan to put your patch on the tip of the branch - I may create > > two branches, one with old and one with new version of the patch since > > when I am testing latest cifs patches (and also the proposed SMB3 > > Posix extensions) don't have Al's series. > > > > On Fri, May 23, 2014 at 9:44 AM, Jeff Layton <jlayton@poochiereds.net> wrote: > >> On Fri, 23 May 2014 06:50:21 -0400 > >> Jeff Layton <jlayton@poochiereds.net> wrote: > >> > >>> Before satisfying a read with cache=loose, we should always check > >>> that the pagecache is valid before allowing a read to be satisfied > >>> out of it. > >>> > >>> Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > >>> Signed-off-by: Jeff Layton <jlayton@poochiereds.net> > >>> --- > >>> fs/cifs/cifsfs.c | 17 +++++++++++++++-- > >>> 1 file changed, 15 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c > >>> index 2c90d07c0b3a..888398067420 100644 > >>> --- a/fs/cifs/cifsfs.c > >>> +++ b/fs/cifs/cifsfs.c > >>> @@ -725,6 +725,19 @@ out_nls: > >>> goto out; > >>> } > >>> > >>> +static ssize_t > >>> +cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter) > >>> +{ > >>> + ssize_t rc; > >>> + struct inode *inode = file_inode(iocb->ki_filp); > >>> + > >>> + rc = cifs_revalidate_mapping(inode); > >>> + if (rc) > >>> + return rc; > >>> + > >>> + return generic_file_read_iter(iocb, iter); > >>> +} > >>> + > >>> static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) > >>> { > >>> struct inode *inode = file_inode(iocb->ki_filp); > >>> @@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = { > >>> const struct file_operations cifs_file_ops = { > >>> .read = new_sync_read, > >>> .write = new_sync_write, > >>> - .read_iter = generic_file_read_iter, > >>> + .read_iter = cifs_loose_read_iter, > >>> .write_iter = cifs_file_write_iter, > >>> .open = cifs_open, > >>> .release = cifs_close, > >>> @@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = { > >>> const struct file_operations cifs_file_nobrl_ops = { > >>> .read = new_sync_read, > >>> .write = new_sync_write, > >>> - .read_iter = generic_file_read_iter, > >>> + .read_iter = cifs_loose_read_iter, > >>> .write_iter = cifs_file_write_iter, > >>> .open = cifs_open, > >>> .release = cifs_close, > >> > >> Steve, > >> > >> This patch is a replacement for the last patch in the 4 patch series > >> for handling reads when cache=loose. The reason for the respin is that > >> aio_read has been replaced by read_iter in linux-next, so this is what > >> we'll want for v3.16 (once Al's read_iter patches are merged). > >> > >> Thanks, > >> -- > >> Jeff Layton <jlayton@poochiereds.net> > > > > > > > > -- > > Thanks, > > > > Steve > > >
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 2c90d07c0b3a..888398067420 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -725,6 +725,19 @@ out_nls: goto out; } +static ssize_t +cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter) +{ + ssize_t rc; + struct inode *inode = file_inode(iocb->ki_filp); + + rc = cifs_revalidate_mapping(inode); + if (rc) + return rc; + + return generic_file_read_iter(iocb, iter); +} + static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) { struct inode *inode = file_inode(iocb->ki_filp); @@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = { const struct file_operations cifs_file_ops = { .read = new_sync_read, .write = new_sync_write, - .read_iter = generic_file_read_iter, + .read_iter = cifs_loose_read_iter, .write_iter = cifs_file_write_iter, .open = cifs_open, .release = cifs_close, @@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = { const struct file_operations cifs_file_nobrl_ops = { .read = new_sync_read, .write = new_sync_write, - .read_iter = generic_file_read_iter, + .read_iter = cifs_loose_read_iter, .write_iter = cifs_file_write_iter, .open = cifs_open, .release = cifs_close,
Before satisfying a read with cache=loose, we should always check that the pagecache is valid before allowing a read to be satisfied out of it. Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: Jeff Layton <jlayton@poochiereds.net> --- fs/cifs/cifsfs.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)