Re: [PATCH] mm: mark async iocb read as NOWAIT once some data has been, copied

From: Kent Overstreet <kent.overstreet@gmail.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Matthew Wilcox <willy@infradead.org>,
	linux-mm <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>
Subject: Re: [PATCH] mm: mark async iocb read as NOWAIT once some data has been, copied
Date: Sat, 17 Oct 2020 15:13:20 -0400	[thread overview]
Message-ID: <20201017191320.GA2198928@zaphod.evilpiepirate.org> (raw)
In-Reply-To: <e8850ae5-2c9e-4c5b-0aed-893577a83512@kernel.dk>

On Sat, Oct 17, 2020 at 11:30:47AM -0600, Jens Axboe wrote:
> On 10/17/20 10:28 AM, Matthew Wilcox wrote:
> > On Sat, Oct 17, 2020 at 09:30:59AM -0600, Jens Axboe wrote:
> >> Once we've copied some data for an iocb that is marked with IOCB_WAITQ,
> >> we should no longer attempt to async lock a new page. Instead make sure
> >> we return the copied amount, and let the caller retry, instead of
> >> returning -EIOCBQUEUED for a new page.
> >>
> >> This should only be possible with read-ahead disabled on the below
> >> device, and multiple threads racing on the same file. Haven't been able
> >> to reproduce on anything else.
> > 
> > Wouldn't this do the job just as well?
> > 
> > @@ -2211,6 +2211,8 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb,
> >  
> >                 put_page(page);
> >                 written += ret;
> > +               if (iocb->ki_flags & IOCB_WAITQ)
> > +                       iocb->ki_flags |= IOCB_NOWAIT;
> >                 if (!iov_iter_count(iter))
> >                         goto out;
> >                 if (ret < nr) {
> 
> Indeed, that's cleaner. Let me re-test with that just to be on the safe
> side, and I'll post a new one.

generic_file_buffered_read() has written passed in, which can be nonzero (DIO
fallback) - we probably want the check to be at the top of the loop.