Re: [PATCH 09/22] Calculate the alloc_flags for allocation only once

Andrew Morton
2009-04-23T22:58:42Z

On Wed, 22 Apr 2009 14:53:14 +0100
Mel Gorman <mel@csn.ul.ie> wrote:

> Factor out the mapping between GFP and alloc_flags only once. Once factored
> out, it only needs to be calculated once but some care must be taken.
>
> [neilb@suse.de says]
> As the test:
>
> - if (((p->flags & PF_MEMALLOC) || unlikely(test_thread_flag(TIF_MEMDIE)))
> - && !in_interrupt()) {
> - if (!(gfp_mask & __GFP_NOMEMALLOC)) {
>
> has been replaced with a slightly weaker one:
>
> + if (alloc_flags & ALLOC_NO_WATERMARKS) {
>
> Without care, this would allow recursion into the allocator via direct
> reclaim. This patch ensures we do not recurse when PF_MEMALLOC is set
> but TF_MEMDIE callers are now allowed to directly reclaim where they
> would have been prevented in the past.
>
> ...
>
> +static inline int
> +gfp_to_alloc_flags(gfp_t gfp_mask)
> +{
> + struct task_struct *p = current;
> + int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
> + const gfp_t wait = gfp_mask & __GFP_WAIT;
> +
> + /*
> + * The caller may dip into page reserves a bit more if the caller
> + * cannot run direct reclaim, or if the caller has realtime scheduling
> + * policy or is asking for __GFP_HIGH memory. GFP_ATOMIC requests will
> + * set both ALLOC_HARDER (!wait) and ALLOC_HIGH (__GFP_HIGH).
> + */
> + if (gfp_mask & __GFP_HIGH)
> + alloc_flags |= ALLOC_HIGH;
> +
> + if (!wait) {
> + alloc_flags |= ALLOC_HARDER;
> + /*
> + * Ignore cpuset if GFP_ATOMIC (!wait) rather than fail alloc.
> + * See also cpuset_zone_allowed() comment in kernel/cpuset.c.
> + */
> + alloc_flags &= ~ALLOC_CPUSET;
> + } else if (unlikely(rt_task(p)))
> + alloc_flags |= ALLOC_HARDER;
> +
> + if (likely(!(gfp_mask & __GFP_NOMEMALLOC))) {
> + if (!in_interrupt() &&
> + ((p->flags & PF_MEMALLOC) ||
> + unlikely(test_thread_flag(TIF_MEMDIE))))
> + alloc_flags |= ALLOC_NO_WATERMARKS;
> + }
> +
> + return alloc_flags;
> +}

hm. Was there a particular reason for the explicit inline?

It's OK as it stands, but might become suboptimal if we later add a
second caller?