Re: [PATCH] gfp: GFP_RECLAIM_MASK should include __GFP_NO_KSWAPD

[Michal Hocko]

On Wed 14-10-15 13:58:05, Pan Xinhui wrote:
> Hi, all
> 	I am working on some debug features' development.
> I use kmalloc in some places of *scheduler*.

This sounds inherently dangerous.

> And the gfp_flag is GFP_ATOMIC, code looks like 
> p = kmalloc(sizeof(*p), GFP_ATOMIC);
> 
> however I notice GFP_ATOMIC is still not enough. because when system
> is at low memory state, slub might try to wakeup kswapd. then some
> weird issues hit.

gfp flags have been reworked in the current mmotm tree so you want
__GFP_ATOMIC here. This will be non sleeping allocation which won't even
wake up kswapd. I guess you do not want/need to touch memory reserves
for something like a debugging feature (so you do not have to abuse
__GFP_HIGH)

[...]

> After some simple check, I change my codes. this time code looks like:
> p = kmalloc(sizeof(*p), GFP_ATOMIC | __GFP_NO_KSWAPD);
> I think this flag will forbid slub to call any scheduler codes. But issue still hit. :(
> 
> my test result shows that __GFP_NO_KSWAPD is cleared when slub pass gfp_flag to page allocator!!!
> 
> at last I found it is clear by codes below.
> 1441 static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
> 1442 {
> 1443         if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
> 1444                 pr_emerg("gfp: %u\n", flags & GFP_SLAB_BUG_MASK);
> 1445                 BUG();
> 1446         }
> 1447 
> 1448         return allocate_slab(s,
> 1449                 flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node);//all other flags will be cleared. my god!!!
> 1450 }
> 
> I think GFP_RECLAIM_MASK should include as many available flags as possible. :)

Not really. It should only contain those which are really reclaim
related. The fact that SLUB drops other flags is an internal detail
of the allocator. If the resulting memory doesn't match the original
requirements (e.g. zone placing etc...) then it is certainly a bug
but not a bug in GFP_RECLAIM_MASK.

Anyway you are right that GFP_RECLAIM_MASK should contain
__GFP_NO_KSWAPD resp. its new representation which is the case in the
current mmotm tree as pointed out in previous response.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/