Re: [PATCH] PageCompound avoid page[1].mapping

[David Gibson]

On Fri, Dec 09, 2005 at 09:56:42PM +0000, Hugh Dickins wrote:
> If a compound page has its own put_page_testzero destructor (the only
> current example is free_huge_page), that is noted in page[1].mapping of
> the compound page.  But David Gibson's recent fix to access_process_vm
> shows that to be rather a poor place to keep it: functions which call
> set_page_dirty(_lock) after get_user_pages ought to check !PageCompound
> first, otherwise set_page_dirty may crash on what's not the address of
> a struct address_space; but Infiniband for one is unaware of this issue.
> 
> Even if we fixed all callers, or set_page_dirty(_lock) itself, it would
> still be unsatisfactory: e.g. get_user_pages calls flush_dcache_page,
> which involves page->mapping on some architectures - not a problem while
> hugetlb goes its own way in get_user_pages, but needs a test if another
> compound page destructor were added.  page->mapping is used too widely
> to be a safe field to reuse in this way.
> 
> The safest field to reuse, given how PageCompound redirects callers to
> the page count of the first page, is actually the _count field of the
> second page: save order (only used for debug) there, and move destructor
> address from mapping to index.  Add __page_count inline for internal
> debug use - to avoid reliance on page_private when page is in doubt.

It's not clear to me this is a good way to go.  It will make things
work neatly in the case where the correct action on a compound page is
to do nothing (and we already have a mapping == NULL test).  However
it will fail silently in cases where we actually need to get at the
mapping for a compound page (so we need to check for CompoundPage and
find the master page).  The existing code will probably blow up in
that case, at least showing there's a problem.

> Revert David's mod to access_process_vm, no longer required.  But leave
> the PageCompound tests in fs/bio.c and fs/direct-io.c: perhaps those are
> worthwhile optimizations when working on hugetlb areas.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/