linux-kernel - Re: [PATCH 1/11] Add __GFP_MOVABLE flag and update callers

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: <Pine.LNX.4.64.0611260039070.27769@blonde.wat.veritas.com>
Date:	Sun, 26 Nov 2006 00:44:59 +0000 (GMT)
From:	Hugh Dickins <hugh@...itas.com>
To:	Linus Torvalds <torvalds@...l.org>
cc:	Mel Gorman <mel@...net.ie>, Christoph Lameter <clameter@....com>,
	linux-mm@...ck.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/11] Add __GFP_MOVABLE flag and update callers

On Sat, 25 Nov 2006, Linus Torvalds wrote:
> On Fri, 24 Nov 2006, Hugh Dickins wrote:
> > 
> > You need to add in something like the patch below (mutatis mutandis
> > for whichever approach you end up taking): tmpfs uses highmem pages
> > for its swap vector blocks, noting where on swap the data pages are,
> > and allocates them with mapping_gfp_mask(inode->i_mapping); but we
> > don't have any mechanism in place for reclaiming or migrating those.
> 
> I think this really just points out that you should _not_ put MOVABLE into 
> the "mapping_gfp_mask()" at all.
> 
> The mapping_gfp_mask() should really just contain the "constraints" on 
> the allocation, not the "how the allocation is used". So things like "I 
> need all my pages to be in the 32bit DMA'able region" is a constraint on 
> the allocator, as is something like "I need the allocation to be atomic". 
> 
> But MOVABLE is really not a constraint on the allocator, it's a guarantee 
> by the code _calling_ the allocator that it will then make sure that it 
> _uses_ the allocation in a way that means that it is movable.
> 
> So it shouldn't be a property of the mapping itself, it should always be a 
> property of the code that actually does the allocation.
> 
> Hmm?

Not anything I feel strongly about, but I don't see it that way.

mapping_gfp_mask() seems to me nothing more than a pragmatic way
of getting the appropriate gfp_mask down to page_cache_alloc().

alloc_inode() initializes it to whatever suits most filesystems
(currently GFP_HIGHUSER), and those who differ adjust it (e.g.
block_dev has good reason to avoid highmem so sets it to GFP_USER
instead).  It used to be the case that several filesystems lacked
kmap() where needed, and those too would set GFP_USER: what you call
a constraint seems to me equally a property of the surrounding code.

If __GFP_MOVABLE is coming in, and most fs's are indeed allocating
movable pages, then I don't see why MOVABLE shouldn't be in the
mapping_gfp_mask.  Specifying MOVABLE constrains both the caller's
use of the pages, and the way they are allocated; as does HIGHMEM.

And we shouldn't be guided by the way tmpfs (ab?)uses that gfp_mask
for its metadata allocations as well as its page_cache_alloc()s:
that's just a special case.  Though the ramfs case is more telling
(its pagecache pages being not at present movable).

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/