From: Chuck Lever <chuck.lever@oracle.com>
To: mgorman@techsingularity.net
Cc: linux-nfs@vger.kernel.org, linux-mm@kvack.org, brouer@redhat.com
Subject: [PATCH v3] mm/page_alloc: Further fix __alloc_pages_bulk() return value
Date: Tue, 29 Jun 2021 09:48:15 -0400 [thread overview]
Message-ID: <162497449506.16614.7781489905877008435.stgit@klimt.1015granger.net> (raw)
The author of commit b3b64ebd3822 ("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.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
mm/page_alloc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e7af86e1a312..49eb2e134f9d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5059,7 +5059,7 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
int nr_populated = 0;
if (unlikely(nr_pages <= 0))
- return 0;
+ goto out;
/*
* Skip populated array elements to determine if any pages need
@@ -5070,7 +5070,7 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
/* 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)
@@ -5080,7 +5080,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 0;
+ goto out;
gfp = alloc_gfp;
/* Find an allowed local zone that meets the low watermark. */
@@ -5153,6 +5153,7 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
local_irq_restore(flags);
+out:
return nr_populated;
failed_irq:
@@ -5168,7 +5169,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);
next reply other threads:[~2021-06-29 13:48 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-29 13:48 Chuck Lever [this message]
2021-06-29 15:33 ` Mel Gorman
2021-06-29 16:04 ` Jesper Dangaard Brouer
2021-06-29 16:01 ` Jesper Dangaard Brouer
2021-06-29 16:32 ` Chuck Lever III
2021-06-30 6:58 ` Mel Gorman
2021-06-30 11:22 ` Jesper Dangaard Brouer
2021-06-30 12:05 ` Mel Gorman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=162497449506.16614.7781489905877008435.stgit@klimt.1015granger.net \
--to=chuck.lever@oracle.com \
--cc=brouer@redhat.com \
--cc=linux-mm@kvack.org \
--cc=linux-nfs@vger.kernel.org \
--cc=mgorman@techsingularity.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox