Re: [RFC] Free up page->private for compound pages

[Hugh Dickins <hugh@veritas.com>]

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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>