Skip to content

Commit

Permalink
mm/page_alloc: further fix __alloc_pages_bulk() return value
Browse files Browse the repository at this point in the history
The author of commit b3b64eb ("mm/page_alloc: do bulk array
bounds check after checking populated elements") was possibly
confused by the mixture of return values throughout the function.

The API contract is clear that the function "Returns the number of pages
on the list or array." It does not list zero as a unique return value with
a special meaning.  Therefore zero is a plausible return value only if
@nr_pages is zero or less.

Clean up the return logic to make it clear that the returned value is
always the total number of pages in the array/list, not the number of
pages that were allocated during this call.

The only change in behavior with this patch is the value returned if
prepare_alloc_pages() fails.  To match the API contract, the number of
pages currently in the array/list is returned in this case.

The call site in __page_pool_alloc_pages_slow() also seems to be confused
on this matter.  It should be attended to by someone who is familiar with
that code.

[mel@techsingularity.net: Return nr_populated if 0 pages are requested]

Link: https://lkml.kernel.org/r/20210713152100.10381-4-mgorman@techsingularity.net
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
Cc: Zhang Qiang <Qiang.Zhang@windriver.com>
Cc: Yanfei Xu <yanfei.xu@windriver.com>
Cc: Matteo Croce <mcroce@microsoft.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Chuck Lever authored and Linus Torvalds committed Jul 15, 2021
1 parent e5c15ce commit 0614784
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions mm/page_alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -5221,19 +5221,20 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
unsigned int alloc_flags = ALLOC_WMARK_LOW;
int nr_populated = 0, nr_account = 0;

if (unlikely(nr_pages <= 0))
return 0;

/*
* Skip populated array elements to determine if any pages need
* to be allocated before disabling IRQs.
*/
while (page_array && nr_populated < nr_pages && page_array[nr_populated])
nr_populated++;

/* No pages requested? */
if (unlikely(nr_pages <= 0))
goto out;

/* Already populated array? */
if (unlikely(page_array && nr_pages - nr_populated == 0))
return nr_populated;
goto out;

/* Use the single page allocator for one page. */
if (nr_pages - nr_populated == 1)
Expand All @@ -5255,7 +5256,7 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
gfp &= gfp_allowed_mask;
alloc_gfp = gfp;
if (!prepare_alloc_pages(gfp, 0, preferred_nid, nodemask, &ac, &alloc_gfp, &alloc_flags))
return nr_populated;
goto out;
gfp = alloc_gfp;

/* Find an allowed local zone that meets the low watermark. */
Expand Down Expand Up @@ -5323,6 +5324,7 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
__count_zid_vm_events(PGALLOC, zone_idx(zone), nr_account);
zone_statistics(ac.preferred_zoneref->zone, zone, nr_account);

out:
return nr_populated;

failed_irq:
Expand All @@ -5338,7 +5340,7 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
nr_populated++;
}

return nr_populated;
goto out;
}
EXPORT_SYMBOL_GPL(__alloc_pages_bulk);

Expand Down

0 comments on commit 0614784

Please sign in to comment.