Re: [PATCH] mm: Warn about costly page allocation

[Mel Gorman]

On Mon, Jul 09, 2012 at 09:50:48PM +0900, Minchan Kim wrote:
> > <SNIP>
> > 
> > You're aiming this at embedded QA people according to your changelog so
> > do whatever you think is going to be the most effective. It's already
> > "known" that high-order kernel allocations are meant to be unreliable and
> > apparently this is being ignored. The in-code warning could look
> > something like
> > 
> > if (unlikely(order > PAGE_ALLOC_COSTLY_ORDER)) {
> > 	printk_once("%s: page allocation high-order stupidity: order:%d, mode:0x%x\n",
> >                    current->comm, order, gfp_mask);
> > 	if (gfp_flags & __GFP_MOVABLE) {
> > 		printk_once("Enable compaction or whatever\n");
> > 		dump_stack();
> > 	} else {
> > 		printk_once("Regular high-order kernel allocations like this will eventually start failing.");
> > 		dump_stack();
> > 	}
> > }
> 
> I'm not sure we have to check further for __GFP_MOVABLE because I have not seen driver
> uses __GFP_MOVABLE for high order allocation. Although it uses the flag, it's never
> compactable since it's out of LRU list. So I think it's rather overkill.
> 

Then I would have considered it even more important to warn them that
their specific usage is going to break eventually, with or without
compaction. However, you know the target audience for this warning so it's
your call.

> > 
> > There should be a comment above it giving more information if you think
> > the embedded people will actually read it. Of course, if this warning
> > triggers during driver initialisation then it might be a completely useless.
> > You could rate limit the warning (printk_ratelimit()) instead to be more
> > effective. As I don't know what sort of device drivers you are seeing this
> > problem with I can't judge what the best style of warning would be.
> 
> Okay.
> I will send patch like below tomorrow if there isn't any objection.
> 
> if (unlikely(order > PAGE_ALLOC_COSTLY_ORDER)) {
> 	if (printk_ratelimit()) {
> 		printk("%s: page allocation high-order stupidity: order:%d, mode:0x%x\n",
> 			current->comm, order, gfp_mask);
> 		printk_once("Enable compaction or whatever\n");
> 		printk_once("Regular high-order kernel allocations like this will eventually start failing.\n");
> 		dump_stack();
> 	}
> }

The warning message could be improved. I did not expect you to use "Enable
compaction or whatever" verbatim. I was just illustrating what type of
warnings I thought might be useful. I expected you would change it to
something that embedded driver authors would pay attention to :)

As you are using printk_ratelimit(), you can also use pr_warning to
annotate this as KERN_WARNING.

-- 
Mel Gorman
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/