| Message ID | 163111666635.283156.177701903478910460.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | afs: Fixes for 3rd party-induced data corruption | expand |
On Wed, Sep 8, 2021 at 12:58 PM David Howells <dhowells@redhat.com> wrote: > > There's a loop in afs_extend_writeback() that adds extra pages to a write > we want to make to improve the efficiency of the writeback by making it > larger. This loop stops, however, if we hit a page we can't write back > from immediately, but it doesn't get rid of the page ref we speculatively > acquired. > > This was caused by the removal of the cleanup loop when the code switched > from using find_get_pages_contig() to xarray scanning as the latter only > gets a single page at a time, not a batch. > > Fix this by putting the page on a ref on an early break from the loop. > Unfortunately, we can't just add that page to the pagevec we're employing > as we'll go through that and add those pages to the RPC call. > > This was found by the generic/074 test. It leaks ~4GiB of RAM each time it > is run - which can be observed with "top". > > Fixes: e87b03f5830e ("afs: Prepare for use of THPs") > Reported-by: Marc Dionne <marc.dionne@auristor.com> > Signed-off-by: David Howells <dhowells@redhat.com> > cc: linux-afs@lists.infradead.org > --- > > fs/afs/write.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/fs/afs/write.c b/fs/afs/write.c > index c0534697268e..66b235266893 100644 > --- a/fs/afs/write.c > +++ b/fs/afs/write.c > @@ -471,13 +471,18 @@ static void afs_extend_writeback(struct address_space *mapping, > } > > /* Has the page moved or been split? */ > - if (unlikely(page != xas_reload(&xas))) > + if (unlikely(page != xas_reload(&xas))) { > + put_page(page); > break; > + } > > - if (!trylock_page(page)) > + if (!trylock_page(page)) { > + put_page(page); > break; > + } > if (!PageDirty(page) || PageWriteback(page)) { > unlock_page(page); > + put_page(page); > break; > } > > @@ -487,6 +492,7 @@ static void afs_extend_writeback(struct address_space *mapping, > t = afs_page_dirty_to(page, priv); > if (f != 0 && !new_content) { > unlock_page(page); > + put_page(page); > break; > } > > > > Reviewed-By: Marc Dionne <marc.dionne@auristor.com> Tested-By: Marc Dionne <marc.dionne@auristor.com> Marc
diff --git a/fs/afs/write.c b/fs/afs/write.c index c0534697268e..66b235266893 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -471,13 +471,18 @@ static void afs_extend_writeback(struct address_space *mapping, } /* Has the page moved or been split? */ - if (unlikely(page != xas_reload(&xas))) + if (unlikely(page != xas_reload(&xas))) { + put_page(page); break; + } - if (!trylock_page(page)) + if (!trylock_page(page)) { + put_page(page); break; + } if (!PageDirty(page) || PageWriteback(page)) { unlock_page(page); + put_page(page); break; } @@ -487,6 +492,7 @@ static void afs_extend_writeback(struct address_space *mapping, t = afs_page_dirty_to(page, priv); if (f != 0 && !new_content) { unlock_page(page); + put_page(page); break; }
There's a loop in afs_extend_writeback() that adds extra pages to a write we want to make to improve the efficiency of the writeback by making it larger. This loop stops, however, if we hit a page we can't write back from immediately, but it doesn't get rid of the page ref we speculatively acquired. This was caused by the removal of the cleanup loop when the code switched from using find_get_pages_contig() to xarray scanning as the latter only gets a single page at a time, not a batch. Fix this by putting the page on a ref on an early break from the loop. Unfortunately, we can't just add that page to the pagevec we're employing as we'll go through that and add those pages to the RPC call. This was found by the generic/074 test. It leaks ~4GiB of RAM each time it is run - which can be observed with "top". Fixes: e87b03f5830e ("afs: Prepare for use of THPs") Reported-by: Marc Dionne <marc.dionne@auristor.com> Signed-off-by: David Howells <dhowells@redhat.com> cc: linux-afs@lists.infradead.org --- fs/afs/write.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)