From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Thu, 20 Nov 2008 18:58:49 +0000 (GMT) From: Hugh Dickins Subject: Re: [PATCH 3/7] mm: remove GFP_HIGHUSER_PAGECACHE In-Reply-To: <20081120164304.GA9777@csn.ul.ie> Message-ID: References: <20081120164304.GA9777@csn.ul.ie> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org Return-Path: To: Mel Gorman Cc: Andrew Morton , linux-mm@kvack.org List-ID: On Thu, 20 Nov 2008, Mel Gorman wrote: > On Thu, Nov 20, 2008 at 01:16:16AM +0000, Hugh Dickins wrote: > > GFP_HIGHUSER_PAGECACHE is just an alias for GFP_HIGHUSER_MOVABLE, > > making that harder to track down: remove it, and its out-of-work > > brothers GFP_NOFS_PAGECACHE and GFP_USER_PAGECACHE. > > The use of GFP_HIGHUSER_PAGECACHE instead of GFP_HIGHUSER_MOVABLE was a > deliberate decision at the time. I do not have an exact patch to point I realize it didn't happen by accident! > you at but the intention behind GFP_HIGHUSER_PAGECACHE was that it be > self-documenting. i.e. one could easily find what GFP placement decisions > have been made for page-cache allocations. I see it as self-obscuring, not self-documenting: of course pagecache pages will normally be allocated with the GFP mask for pagecache pages, but what is that? Ah, it's GFP_HIGHUSER_MOVABLE. Please let's not go down the road, I mean, let's retrace our steps up the road, of assigning a unique GFP name for every use of pages. > So, I'm happy with GFP_NOFS_PAGECACHE and GFP_USER_PAGECACHE going away and > it makes perfect sense. GFP_HIGHUSER_PAGECACHE I'm not as keen on backing > out. I like it's self-documenting aspect but aliases sometimes make peoples > teeth itch. (No, what made my teeth itch was "is this safe?" in memory.c ;) > If it's really hated, then could a comment to the affect of > "Marked movable for a page cache allocation" be placed near the call-sites > instead? I'd prefer not. The only places where GFP_HIGHUSER_PAGECACHE appeared were the mapping_set_gfp_mask when initializing an inode, and hotremove_migrate_alloc(). The latter allocating for anonymous pages also, like most places where GFP_HIGHUSER_MOVABLE is specified. But I'd better not complain that it's not obvious to me which should be marked with your comment and which not: you'll point to that as evidence that we're missing out on the self-documentation! Perhaps the problem is that nobody else has been following your lead. Hugh -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org