This patch was inadvertently included. I blame my tools. The actual patch 2/3 is the other one. On Tue, Oct 13, 2020 at 04:00:06AM +0100, Matthew Wilcox (Oracle) wrote: > In the upcoming THP patch series, if we find a !Uptodate page, it > is because of a read error. In this case, we want to split the THP > into smaller pages so we can handle the error in as granular a fashion > as possible. But xfstests generic/273 defeats this strategy by having > 500 threads all sleeping on the same page, each waiting for their turn > to split the page. None of them will ever succeed because splitting a > page requires that you hold the only reference to it. > > To fix this, use put_and_wait_on_page_locked() to sleep without holding > a reference. Each of the readers will then go back and retry the > page lookup. > > This requires a few changes since we now get the page lock a little > earlier in generic_file_buffered_read(). This is unlikely to affect any > normal workloads as pages in the page cache are generally uptodate and > will not hit this path. With the THP patch set and the readahead error > injector, I see about a 25% performance improvement with this patch over > an alternate approach which moves the page locking down. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx> > --- > mm/filemap.c | 51 ++++++++++++++------------------------------------- > 1 file changed, 14 insertions(+), 37 deletions(-) > > diff --git a/mm/filemap.c b/mm/filemap.c > index f70227941627..9916353f0f0d 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1254,14 +1254,6 @@ static int __wait_on_page_locked_async(struct page *page, > return ret; > } > > -static int wait_on_page_locked_async(struct page *page, > - struct wait_page_queue *wait) > -{ > - if (!PageLocked(page)) > - return 0; > - return __wait_on_page_locked_async(compound_head(page), wait, false); > -} > - > /** > * put_and_wait_on_page_locked - Drop a reference and wait for it to be unlocked > * @page: The page to wait for. > @@ -2128,19 +2120,21 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, > put_page(page); > goto out; > } > - error = wait_on_page_locked_async(page, > - iocb->ki_waitq); > + error = lock_page_async(page, iocb->ki_waitq); > + if (error) > + goto readpage_error; > + } else if (iocb->ki_flags & IOCB_NOWAIT) { > + put_page(page); > + goto would_block; > } else { > - if (iocb->ki_flags & IOCB_NOWAIT) { > - put_page(page); > - goto would_block; > + if (!trylock_page(page)) { > + put_and_wait_on_page_locked(page, > + TASK_KILLABLE); > + goto find_page; > } > - error = wait_on_page_locked_killable(page); > } > - if (unlikely(error)) > - goto readpage_error; > if (PageUptodate(page)) > - goto page_ok; > + goto uptodate; > > if (inode->i_blkbits == PAGE_SHIFT || > !mapping->a_ops->is_partially_uptodate) > @@ -2148,14 +2142,13 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, > /* pipes can't handle partially uptodate pages */ > if (unlikely(iov_iter_is_pipe(iter))) > goto page_not_up_to_date; > - if (!trylock_page(page)) > - goto page_not_up_to_date; > /* Did it get truncated before we got the lock? */ > if (!page->mapping) > - goto page_not_up_to_date_locked; > + goto page_not_up_to_date; > if (!mapping->a_ops->is_partially_uptodate(page, > offset, iter->count)) > - goto page_not_up_to_date_locked; > + goto page_not_up_to_date; > +uptodate: > unlock_page(page); > } > page_ok: > @@ -2223,28 +2216,12 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, > continue; > > page_not_up_to_date: > - /* Get exclusive access to the page ... */ > - if (iocb->ki_flags & IOCB_WAITQ) > - error = lock_page_async(page, iocb->ki_waitq); > - else > - error = lock_page_killable(page); > - if (unlikely(error)) > - goto readpage_error; > - > -page_not_up_to_date_locked: > /* Did it get truncated before we got the lock? */ > if (!page->mapping) { > unlock_page(page); > put_page(page); > continue; > } > - > - /* Did somebody else fill it already? */ > - if (PageUptodate(page)) { > - unlock_page(page); > - goto page_ok; > - } > - > readpage: > if (iocb->ki_flags & (IOCB_NOIO | IOCB_NOWAIT)) { > unlock_page(page); > -- > 2.28.0 >