linux-kernel - Re: [PATCH v2] 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]

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

On Tue, 10 Jul 2012 08:55:53 +0900
Minchan Kim <minchan@...nel.org> wrote:

> ...
>
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2276,6 +2276,29 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
>  	return alloc_flags;
>  }
>  
> +#if defined(CONFIG_DEBUG_VM) && !defined(CONFIG_COMPACTION)
> +static inline void check_page_alloc_costly_order(unsigned int order, gfp_t flags)
> +{
> +	if (likely(order <= PAGE_ALLOC_COSTLY_ORDER))
> +		return;
> +
> +	if (!printk_ratelimited())
> +		return;
> +
> +	pr_warn("%s: page allocation high-order stupidity: "
> +		"order:%d, mode:0x%x\n", current->comm, order, flags);
> +	pr_warn("Enable compaction if high-order allocations are "
> +		"very few and rare.\n");
> +	pr_warn("If you need regular high-order allocation, "
> +		"compaction wouldn't help it.\n");
> +	dump_stack();
> +}
> +#else
> +static inline void check_page_alloc_costly_order(unsigned int order)
> +{
> +}
> +#endif

Let's remember that plain old "inline" is ignored by the compiler.  If
we really really want to inline something then we should use
__always_inline.

And inlining this function would be a bad thing to do - it causes the
outer function to have an increased cache footprint.  A good way to
optimise this function is probably to move the unlikely stuff
out-of-line:

	if (unlikely(order > PAGE_ALLOC_COSTLY_ORDER))
		check_page_alloc_costly_order(...);

or

static noinline void __check_page_alloc_costly_order(...)
{
}

static __always_inline void check_page_alloc_costly_order(...)
{
	if (unlikely(order > PAGE_ALLOC_COSTLY_ORDER))
		__check_page_alloc_costly_order(...);
}

Also, the displayed messages don't seem very, umm, professional.  Who
was stupid - us or the kernel-configurer?  And "Enable
CONFIG_COMPACTION" would be more specific (and hence helpful) than
"Enable compaction").

And how on earth is the user, or the person who is configuring kernels
for customers to determine whether the kernel will be frequently
performing higher-order allocations?

So I dunno, this all looks like we have a kernel problem and we're
throwing our problem onto hopelessly ill-equipped users of that kernel?

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