linux-kernel - Re: [PATCH trivial] block: GFP_ATOMIC is __GFP

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:	Sun, 29 Jun 2008 07:16:49 +0100 (BST)
From:	Hugh Dickins <hugh@...itas.com>
To:	Eduard - Gabriel Munteanu <eduard.munteanu@...ux360.ro>
cc:	Jens Axboe <jens.axboe@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH trivial] block: GFP_ATOMIC is __GFP_HIGH

On Sun, 29 Jun 2008, Eduard - Gabriel Munteanu wrote:
> On Sun, 29 Jun 2008 00:54:27 +0100 (BST)
> Hugh Dickins <hugh@...itas.com> wrote:
> 
> > GFP_ATOMIC is __GFP_HIGH: no need for alloc_io_context() to add that.
> > 
> > -		INIT_RADIX_TREE(&ret->radix_root, GFP_ATOMIC |
> > __GFP_HIGH);
> > +		INIT_RADIX_TREE(&ret->radix_root, GFP_ATOMIC);
> 
> I'm not sure this is a good idea: GFP_ATOMIC and __GFP_HIGH are
> semantically different, even though they are equivalent at the moment.
> Have you seen GFP_NOWAIT's definition?
> /* This equals 0, but use constants in case they ever change */
> #define GFP_NOWAIT      (GFP_ATOMIC & ~__GFP_HIGH)
> 
> I think it's best to look at what that code intends to do, not at what
> it does at the moment. Definitions for gfp flags might change in the
> future.
> 
> If the code does not _semantically_ need __GFP_HIGH, then your commit
> message should indicate so, rather than comparing it with GFP_ATOMIC.

I disagree.  It is somewhat accidental that GFP_ATOMIC sets no other
bit than __GFP_HIGH - there might have been a __GFP_ATOMIC bit - which
is why the GFP_NOWAIT definition makes some sense; but it is not
accidental that GFP_ATOMIC includes __GFP_HIGH - it's precisely when
we're atomic that we need access to those extra reserves; and where
we don't actually want them then we do say GFP_NOWAIT not GFP_ATOMIC.

I expect the gfp flags will change in the future; but unless I missed
somewhere, amongst all the places which specify GFP_ATOMIC throughout
the kernel, this is the only one which ors in __GFP_HIGH too.  I don't
believe it expected access to extra extra reserves!  So I thought we'd
do best to remove the anomaly.

(But what I'd actually intended to grep for was __GFP_HIGHMEM.)

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