linux-kernel - Re: [PATCH] mm: Warn about costly page allocation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120709130551.GA14154@suse.de>
Date:	Mon, 9 Jul 2012 14:05:51 +0100
From:	Mel Gorman <mgorman@...e.de>
To:	Minchan Kim <minchan@...nel.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Rik van Riel <riel@...hat.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Kamezawa Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Subject: Re: [PATCH] mm: Warn about costly page allocation

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/