Messages in this thread | | | From | Pengfei Li <> | Date | Sat, 27 Jul 2019 10:34:04 +0800 | Subject | Re: [PATCH 02/10] mm/page_alloc: use unsigned int for "order" in __rmqueue_fallback() |
| |
On Fri, Jul 26, 2019 at 5:36 PM Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > > On 25/07/2019 20.42, Pengfei Li wrote: > > Because "order" will never be negative in __rmqueue_fallback(), > > so just make "order" unsigned int. > > And modify trace_mm_page_alloc_extfrag() accordingly. > > > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index 75c18f4fd66a..1432cbcd87cd 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -2631,8 +2631,8 @@ static bool unreserve_highatomic_pageblock(const struct alloc_context *ac, > > * condition simpler. > > */ > > static __always_inline bool > > -__rmqueue_fallback(struct zone *zone, int order, int start_migratetype, > > - unsigned int alloc_flags) > > +__rmqueue_fallback(struct zone *zone, unsigned int order, > > + int start_migratetype, unsigned int alloc_flags) > > { > > Please read the last paragraph of the comment above this function, run > git blame to figure out when that was introduced, and then read the full > commit description.
Thanks for your comments.
I have read the commit info of commit b002529d2563 ("mm/page_alloc.c: eliminate unsigned confusion in __rmqueue_fallback").
And I looked at the discussion at https://lkml.org/lkml/2017/6/21/684 in detail.
> Here be dragons. At the very least, this patch is > wrong in that it makes that comment inaccurate.
I wonder if you noticed the commit 6bb154504f8b ("mm, page_alloc: spread allocations across zones before introducing fragmentation").
Commit 6bb154504f8b introduces a local variable min_order in __rmqueue_fallback().
And you can see
for (current_order = MAX_ORDER - 1; current_order >= min_order; --current_order) {
The “current_order” and "min_order" are int, so here is ok.
Since __rmqueue_fallback() is only called by __rmqueue() and "order" is unsigned int in __rmqueue(), then I think that making "order" is also unsigned int is good.
Maybe I should also modify the comments here?
> > Rasmus
Thank you again for your review.
-- Pengfei
| |