Re: [PATCH]: Clean up of __alloc_pages

Rohit Seth
2005-10-31T21:20:54Z

On Sat, 2005-10-29 at 18:47 -0700, Paul Jackson wrote:
> A couple more items:
> 1) Lets try for a consistent use of type "gfp_t" for gfp_mask.
> 2) The can_try_harder flag values were driving me nuts.

Not sure why? Let me see if some new values could better articulate the
meaning. Currently if value is < 0 then don't check the watermarks.
When we do check for watermarks, then the value of 1 indicates that it
could go below minimum value.

> 3) The "inline" you added to buffered_rmqueue() blew up my compile.

I will remove the inline based on your and Nick emails. Though my patch
had inline before the struct....

-static struct page *
-buffered_rmqueue(struct zone *zone, int order, gfp_t gfp_flags)
+static inline struct page *
+buffered_rmqueue(struct zone *zone, int order, gfp_t gfp_flags, int
replenish)

...so that shouldn't have caused any problem.

> 4) The return from try_to_free_pages() was put in "i" for no evident reason.

Will be fixed.

> 5) I have no clue what the replenish flag you added to buffered_rmqueue does.
>

A bit futuristic. Will need it when pcp allocations gets checked first
(as Nick also mentioned). Will remove it for now.

> You patch has:
> > can_try_harder can have following
> > * values:
> > * -1 => No need to check for the watermarks.
> > * 0 => Don't go too low down in deeps below the low watermark (GFP_HIGH)
> > * 1 => Go far below the low watermark. See zone_watermark_ok (RT TASK)
>
> Later on, you have an inequality test on this value:
> if ((can_try_harder >= 0)
> and a non-zero test:
> if (can_try_harder)

The last line is from zone_watermark_ok. The first check is in
get_page_from_freelist. There is no (can_try_harder) check for this
flag in that function.

>
> That's three magic values, not even in increasing order of "how hard
> one should try", tested a couple of different ways that requires
> absorbing the complete details of the three values and their ordering
> before one can read the code.