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);
> >
>
