lkml.org 
[lkml]   [2006]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/11] Add __GFP_MOVABLE flag and update callers
On Sun, 26 Nov 2006, Hugh Dickins wrote:

> 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.
>>

Later, MOVABLE might be a constraint. For example, hotpluggable nodes may
only allow MOVABLE allocations to be allocated.

>> 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().
>

And that is important for any filesystem that uses generic_file_read(). As
page_cache_alloc() has no knowledge of the filesystem, it depends on the
mapping_gfp_mask() to determine if the pages are movable or not.

> 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.
>

From what I've seen, the majority of filesystems are suitable for using
__GFP_MOVABLE and it would be clearer to have GFP_HIGH_MOVABLE as the
default and setting GFP_HIGHUSER in filesystems like ramfs.

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

--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-11-27 17:35    [W:0.080 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site