linux-kernel - Re: [PATCH 4/6] mm: kvmalloc support __GFP_RETRY

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: <80500165-94c2-2d5c-ff7a-6310916da288@suse.cz>
Date:   Mon, 26 Jun 2017 14:00:27 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Michal Hocko <mhocko@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     Johannes Weiner <hannes@...xchg.org>, Mel Gorman <mgorman@...e.de>,
        NeilBrown <neilb@...e.com>, LKML <linux-kernel@...r.kernel.org>,
        linux-mm@...ck.org, Michal Hocko <mhocko@...e.com>
Subject: Re: [PATCH 4/6] mm: kvmalloc support __GFP_RETRY_MAYFAIL for all
 sizes

On 06/23/2017 10:53 AM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@...e.com>
> 
> Now that __GFP_RETRY_MAYFAIL has a reasonable semantic regardless of the
> request size we can drop the hackish implementation for !costly orders.
> __GFP_RETRY_MAYFAIL retries as long as the reclaim makes a forward
> progress and backs of when we are out of memory for the requested size.
> Therefore we do not need to enforce__GFP_NORETRY for !costly orders just
> to silent the oom killer anymore.
> 
> Signed-off-by: Michal Hocko <mhocko@...e.com>

The flag is now supported, but not for the embedded page table
allocations, so OOM is still theoretically possible, right?
That should be rare, though. Worth mentioning anywhere?

Other than that.
Acked-by: Vlastimil Babka <vbabka@...e.cz>

> ---
>  mm/util.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/util.c b/mm/util.c
> index 6520f2d4a226..ee250e2cde34 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -339,9 +339,9 @@ EXPORT_SYMBOL(vm_mmap);
>   * Uses kmalloc to get the memory but if the allocation fails then falls back
>   * to the vmalloc allocator. Use kvfree for freeing the memory.
>   *
> - * Reclaim modifiers - __GFP_NORETRY and __GFP_NOFAIL are not supported. __GFP_RETRY_MAYFAIL
> - * is supported only for large (>32kB) allocations, and it should be used only if
> - * kmalloc is preferable to the vmalloc fallback, due to visible performance drawbacks.
> + * Reclaim modifiers - __GFP_NORETRY and __GFP_NOFAIL are not supported.
> + * __GFP_RETRY_MAYFAIL is supported, and it should be used only if kmalloc is
> + * preferable to the vmalloc fallback, due to visible performance drawbacks.
>   *
>   * Any use of gfp flags outside of GFP_KERNEL should be consulted with mm people.
>   */
> @@ -366,13 +366,7 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
>  	if (size > PAGE_SIZE) {
>  		kmalloc_flags |= __GFP_NOWARN;
>  
> -		/*
> -		 * We have to override __GFP_RETRY_MAYFAIL by __GFP_NORETRY for !costly
> -		 * requests because there is no other way to tell the allocator
> -		 * that we want to fail rather than retry endlessly.
> -		 */
> -		if (!(kmalloc_flags & __GFP_RETRY_MAYFAIL) ||
> -				(size <= PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER))
> +		if (!(kmalloc_flags & __GFP_RETRY_MAYFAIL))
>  			kmalloc_flags |= __GFP_NORETRY;
>  	}
>  
>