Re: [PATCH] Cpuset: alloc_pages_node overrides cpuset constraints

[Paul Jackson <pj@sgi.com>]

Andi wrote:
> Faster would be if (gfp_mask & (__GFP_NOCPUSET|__GFP_HARDWALL)) { /* sort them out */ } 

Yup - good point.  However ...

Note that I am already off the fast path here, having peeled off the
interrupt and node inside cpuset cases above.  After these checks for
__GFP_NOCPUSET or __GFP_HARDWALL, only the rare case of having to
look outside a cpuset with no free memory for essential kernel memory
remains.

Look at this entire routine:

int __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
{
        int node;                       /* node that zone z is on */
        const struct cpuset *cs;        /* current cpuset ancestors */
        int allowed;	                /* is allocation in zone z allowed? */

        if (in_interrupt())
                return 1;
        node = z->zone_pgdat->node_id;
        if (node_isset(node, current->mems_allowed))
                return 1;
        if (gfp_mask & __GFP_NOCPUSET)
                return 1;
        if (gfp_mask & __GFP_HARDWALL)  /* If hardwall request, stop here */
                return 0;

        if (current->flags & PF_EXITING) /* Let dying task have memory */
                return 1;

        /* Not hardwall and node outside mems_allowed: scan up cpusets */
        mutex_lock(&callback_mutex);

        task_lock(current);
        cs = nearest_exclusive_ancestor(current->cpuset);
        task_unlock(current);

        allowed = node_isset(node, cs->mems_allowed);
        mutex_unlock(&callback_mutex);
        return allowed;
}

Notice that if neither of the __GFP_NOCPUSET or __GFP_HARDWALL flag
tests fire, then the code hits a mutex, spinlock and subroutine call.

Also notice that the __GFP_NOCPUSET case is the most important case of
those at or below that check.  Any alloc_pages_node, zmalloc_node or
kmalloc_node call that requires a node outside the cpuset hits that
case.  Only tasks that have used up all the available memory in their
cpuset commonly get past here, to the __GFP_HARDWALL case, which is not
a case worth optimizing at the expense of more important code paths.

So ... actually ... I suspect that doing:

        if (gfp_mask & __GFP_NOCPUSET)
                return 1;
        if (gfp_mask & __GFP_HARDWALL)
                return 0;

is faster than doing:

	if (gfp_mask & (__GFP_NOCPUSET|__GFP_HARDWALL)) {
		if (gfp_mask & __GFP_NOCPUSET)
			return 1;
		if (gfp_mask & __GFP_HARDWALL)
			return 0;
	}

because the first of these two gets to the relatively more important
case of __GFP_NOCPUSET faster.

Too bad the patch didn't show a little more context.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401