linux-kernel - Re: [PATCH] mm: warn about allocations which stall for too long

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:   Fri, 23 Sep 2016 16:29:36 +0800
From:   "Hillf Danton" <hillf.zj@...baba-inc.com>
To:     "'Michal Hocko'" <mhocko@...nel.org>, <linux-mm@...ck.org>
Cc:     "'Andrew Morton'" <akpm@...ux-foundation.org>,
        "'Johannes Weiner'" <hannes@...xchg.org>,
        "'Mel Gorman'" <mgorman@...e.de>,
        "'Tetsuo Handa'" <penguin-kernel@...ove.SAKURA.ne.jp>,
        "'LKML'" <linux-kernel@...r.kernel.org>,
        "'Michal Hocko'" <mhocko@...e.com>
Subject: Re: [PATCH] mm: warn about allocations which stall for too long

> 
> From: Michal Hocko <mhocko@...e.com>
> 
> Currently we do warn only about allocation failures but small
> allocations are basically nofail and they might loop in the page
> allocator for a long time.  Especially when the reclaim cannot make
> any progress - e.g. GFP_NOFS cannot invoke the oom killer and rely on
> a different context to make a forward progress in case there is a lot
> memory used by filesystems.
> 
> Give us at least a clue when something like this happens and warn about
> allocations which take more than 10s. Print the basic allocation context
> information along with the cumulative time spent in the allocation as
> well as the allocation stack. Repeat the warning after every 10 seconds so
> that we know that the problem is permanent rather than ephemeral.
> 
> Signed-off-by: Michal Hocko <mhocko@...e.com>
> ---
> 
> Hi,
> I am sending this as an RFC because I am not really sure what is the reasonable
> timeout when to warn. I went with 10s because that should be close to "for ever"
> from the user perspective. But maybe a shorter would be helpful as well?
> I didn't go with a tunable because I would rather not add a new one.
> 
> Thoughts? Ideas?
> 
>  mm/page_alloc.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5155485057cb..d5faab8aa94d 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3485,6 +3485,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	enum compact_result compact_result;
>  	int compaction_retries = 0;
>  	int no_progress_loops = 0;
> +	unsigned long alloc_start = jiffies;
> +	unsigned int stall_timeout = 10 * HZ;
> 
>  	/*
>  	 * In the slowpath, we sanity check order to avoid ever trying to
> @@ -3659,6 +3661,15 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	else
>  		no_progress_loops++;
> 
> +	/* Make sure we know about allocations which stall for too long */
> +	if (!(gfp_mask & __GFP_NOWARN) && time_after(jiffies, alloc_start + stall_timeout)) {
> +		pr_warn("%s: page alloction stalls for %ums: order:%u mode:%#x(%pGg)\n",
> +				current->comm, jiffies_to_msecs(jiffies-alloc_start),

Better if pid is also printed.
> +				order, gfp_mask, &gfp_mask);
> +		stall_timeout += 10 * HZ;

Alternatively	 alloc_start = jiffies;

> +		dump_stack();
> +	}
> +
>  	if (should_reclaim_retry(gfp_mask, order, ac, alloc_flags,
>  				 did_some_progress > 0, no_progress_loops))
>  		goto retry;
> --
> 2.9.3
> 
thanks
Hillf