linux-kernel - Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __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]

Date:	Fri, 17 Jun 2016 14:22:35 -0400
From:	Johannes Weiner <hannes@...xchg.org>
To:	Michal Hocko <mhocko@...nel.org>
Cc:	Dave Chinner <david@...morbit.com>, linux-mm@...ck.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mel Gorman <mgorman@...e.de>, Vlastimil Babka <vbabka@...e.cz>,
	Rik van Riel <riel@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 2/2] xfs: map KM_MAYFAIL to __GFP_RETRY_HARD

On Thu, Jun 16, 2016 at 01:26:06PM +0200, Michal Hocko wrote:
> @@ -54,6 +54,13 @@ kmem_flags_convert(xfs_km_flags_t flags)
>  			lflags &= ~__GFP_FS;
>  	}
>  
> +	/*
> +	 * Default page/slab allocator behavior is to retry for ever
> +	 * for small allocations. We can override this behavior by using
> +	 * __GFP_RETRY_HARD which will tell the allocator to retry as long
> +	 * as it is feasible but rather fail than retry for ever for all
> +	 * request sizes.
> +	 */
>  	if (flags & KM_MAYFAIL)
>  		lflags |= __GFP_RETRY_HARD;

I think this example shows that __GFP_RETRY_HARD is not a good flag
because it conflates two seemingly unrelated semantics; the comment
doesn't quite make up for that.

When the flag is set,

- it allows costly orders to invoke the OOM killer and retry
- it allows !costly orders to fail

While 1. is obvious from the name, 2. is not. Even if we don't want
full-on fine-grained naming for every reclaim methodology and retry
behavior, those two things just shouldn't be tied together.

I don't see us failing !costly order per default anytime soon, and
they are common, so adding a __GFP_MAYFAIL to explicitely override
that behavior seems like a good idea to me. That would make the XFS
callsite here perfectly obvious.

And you can still combine it with __GFP_REPEAT.

For a generic allocation site like this, __GFP_MAYFAIL | __GFP_REPEAT
does the right thing for all orders, and it's self-explanatory: try
hard, allow falling back.

Whether we want a __GFP_REPEAT or __GFP_TRY_HARD at all is a different
topic. In the long term, it might be better to provide best-effort per
default and simply annotate MAYFAIL/NORETRY callsites that want to
give up earlier. Because as I mentioned at LSFMM, it's much easier to
identify callsites that have a convenient fallback than callsites that
need to "try harder." Everybody thinks their allocations are oh so
important. The former is much more specific and uses obvious criteria.

Either way, __GFP_MAYFAIL should be on its own.