could you resend - also add cc:stable if you think appropriate.
On Mon, Sep 19, 2022 at 11:08 PM Leif Sahlberg <lsahlber@xxxxxxxxxx> wrote:
>
> On Tue, Sep 20, 2022 at 11:43 AM Tom Talpey <tom@xxxxxxxxxx> wrote:
> >
> > On 9/19/2022 5:37 PM, Ronnie Sahlberg wrote:
> > > This is the opposite case of kernel bugzilla 216301.
> > > If we mmap a file using cache=none and then proceed to update the mmapped
> > > area these updates are not reflected in a later pread() of that part of the
> > > file.
> > > To fix this we must first destage any dirty pages in the range before
> > > we allow the pread() to proceed.
> > >
> > > Reviewed-by: Paulo Alcantara (SUSE) <pc@xxxxxx>
> > > Reviewed-by: Enzo Matsumiya <ematsumiya@xxxxxxx>
> > > Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> > > ---
> > > fs/cifs/file.c | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> > > index 6f38b134a346..1f3eb97ef4ab 100644
> > > --- a/fs/cifs/file.c
> > > +++ b/fs/cifs/file.c
> > > @@ -4271,6 +4271,15 @@ static ssize_t __cifs_readv(
> > > len = ctx->len;
> > > }
> > >
> > > + if (direct) {
> > > + rc = filemap_write_and_wait_range(file->f_inode->i_mapping,
> > > + offset, offset + len - 1);
> > > + if (rc) {
> > > + kref_put(&ctx->refcount, cifs_aio_ctx_release);
> > > + return rc;
> >
> > Are the possible return values from filemap_write_and_wait_range() also
> > valid for returning from __cifs_readv()? It seems a bit odd to return
> > write errors here.
> >
> > If not, then perhaps a more generic failure rc would be safer/more POSIX
> > compliant?
>
> Good point. Since an error here means we have dirty pages and
> destaging them failed I guess -EAGAIN
> would be the best to return. We want/need userspace to retry this,
> possibly forever, in that scenario.
>
> Steve, can you change the patch to -EAGAIN or do you want me to re-send it?
>
> regards
> ronnie s
>
>
> >
> > Tom.
> >
> > > + }
> > > + }
> > > +
> > > /* grab a lock here due to read response handlers can access ctx */
> > > mutex_lock(&ctx->aio_mutex);
> > >
> >
>
--
Thanks,
Steve
