From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Thu, 5 Apr 2007 20:50:48 +0100 (BST) From: Hugh Dickins Subject: Re: [RFC] Free up page->private for compound pages In-Reply-To: Message-ID: References: <20070405033648.GG11192@wotan.suse.de> <20070405035741.GH11192@wotan.suse.de> <20070405042502.GI11192@wotan.suse.de> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org Return-Path: To: Christoph Lameter Cc: Nick Piggin , linux-mm@kvack.org, dgc@sgi.com List-ID: On Thu, 5 Apr 2007, Christoph Lameter wrote: > On Thu, 5 Apr 2007, Hugh Dickins wrote: > > > > > off through its page->private (page->first_page comes from another > > > > of your patches, not in -mm). Looks like you need to add a test for > > Yes its in mm. See the slub patches. So it is, sorry, don't know what tree I was mistakenly looking in then. > > > > PageCompound in compound_head (what a surprise!), unfortunately. > > > > > > Hmmm... Thus we should really have separate page flag and not overload it? > > > > Of course that would be more efficient, but is it really something > > we'd want to be spending a page flag on? And it's mainly a codesize > > thing, the initial unlikely(PageCompound) tests should keep the main > > paths as fast as before, shouldn't they? > > I am not so much worried about performance but more about the availability > of the page->private field of compound pages. Yes, I realise that. I meant if (unlikely(PageCompound(page)) && PageTail(page)) shouldn't slow down the !PageCompound fast paths more than the existing if (unlikely(PageCompound(page))) or the if (unlikely(PageTail(page))) you had. > > > But I did wonder whether you could do it differently, but not setting > > PageCompound on the first struct page of the compound at all - that > > one doesn't need the compound page adjustment, of course, which is > > your whole point. > > Have not thought about it being a performance improvement. Good point > though. > > > Then in those places which really need to know the first is compounded, > > test something like PageCompound(page+1) instead. "something like" > > because that particular test won't work nicely for the very last > > struct page in a ... node? (sorry, I don't know the right terminology: > > the last struct page in a mem_map-like array). > > The last page in a MAX_ORDER block may have issues. In particular if its > the last MAX_ORDER block in a zone. This going to make sparsemem go > ballistic. Yes, that was my "something like" point: if the compiler allowed it (I'm sure not!) you'd probably want PageCompound(page|1) instead, where that "1" is following pointer arithmetic ;) If the alignment of a mem_map is nicely guaranteed. > > > But if that ends up peppering the code with PageCompound(page) || > > PageCompound(page+1) expressions on fast paths, it'd be a whole lot > > worse than the PageCompound(page) && PageTail(page) we're envisaging. > > Not sure exactly what you are saying. > > The initial proposal was to have > > > 1. Headpage PageCompound > > 2. Tail page PageCompound & PageTail > > The PageCompound on each page is necessary for various I/O paths that > check for compound pages and refuse to do certain things (like dirtying > etc). Yes, those set_page_dirty_lock places. I think those used to be important, because we put the compound page dtor into page[1].mapping, and set_page_dirty would go crazy when it mistook that for a struct address_space*. When we moved the dtor pointer into page[1].lru.next, I left those tests alone as a minor optimization: it just didn't need to bother with the set_page_dirty. If the first page of the compound was not marked PageCompound, they could be updated or not, they're just not important; but never mind, you probably don't want to go that way. > > The tail marking is advantages because it exactly marks a page that is > > 1. Compound > > 2. Not the head of the compound page > > Thus is easy and fast to establish the need to lookup the head page of a > compound page. > > I think we cannot overload the page flag after all because of the page > count issue you pointed out. Guess I should be cleaning up my > initial patch and repost it? I still think PageTail is not worth its own distinct page flag: I can understand you drawing back from my page+1 suggestion, but I don't understand why you're so reluctant to say if (unlikely(PageCompound(page)) && PageTail(page)) 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