Messages in this thread | | | Date | Thu, 31 Mar 2022 19:35:25 +0200 | From | David Sterba <> | Subject | Re: [PATCH v3 2/2] btrfs: allocate page arrays using bulk page allocator |
| |
On Wed, Mar 30, 2022 at 04:11:23PM -0400, Sweet Tea Dorminy wrote: > While calling alloc_page() in a loop is an effective way to populate an > array of pages, the kernel provides a method to allocate pages in bulk. > alloc_pages_bulk_array() populates the NULL slots in a page array, trying to > grab more than one page at a time. > > Unfortunately, it doesn't guarantee allocating all slots in the array, > but it's easy to call it in a loop and return an error if no progress > occurs. Similar code can be found in xfs/xfs_buf.c:xfs_buf_alloc_pages(). > > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> > Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > Changes in v3: > - Added a newline after variable declaration > Changes in v2: > - Moved from ctree.c to extent_io.c > --- > fs/btrfs/extent_io.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index ab4c1c4d1b59..b268e47aa2b7 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -3144,19 +3144,25 @@ static void end_bio_extent_readpage(struct bio *bio) > */ > int btrfs_alloc_page_array(unsigned long nr_pages, struct page **page_array) > { > - int i; > + long allocated = 0; > + > + for (;;) { > + long last = allocated; > > - for (i = 0; i < nr_pages; i++) { > - struct page *page; > + allocated = alloc_pages_bulk_array(GFP_NOFS, nr_pages, > + page_array); > + if (allocated == nr_pages) > + return 0; > > - if (page_array[i]) > + if (allocated != last) > continue; > - page = alloc_page(GFP_NOFS); > - if (!page) > - return -ENOMEM; > - page_array[i] = page; > + /* > + * During this iteration, no page could be allocated, even > + * though alloc_pages_bulk_array() falls back to alloc_page() > + * if it could not bulk-allocate. So we must be out of memory. > + */ > + return -ENOMEM; > }
I find the way the loop is structured a bit cumbersome so I'd suggest to rewrite it as:
int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array) { unsigned int allocated;
for (allocated = 0; allocated < nr_pages;) { unsigned int last = allocated;
allocated = alloc_pages_bulk_array(GFP_NOFS, nr_pages, page_array);
/* * During this iteration, no page could be allocated, even * though alloc_pages_bulk_array() falls back to alloc_page() * if it could not bulk-allocate. So we must be out of memory. */ if (allocated == last) return -ENOMEM; } return 0; }
Also in the xfs code there's memalloc_retry_wait() which is supposed to be called when repeated memory allocation is retried. What was the reason you removed it?
| |