Hugh Dickins <[email protected]> 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.
>
> 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.
>
> ...
> - page[1].mapping = (void *)free_huge_page;
> + page[1].index = (unsigned long)free_huge_page; /* set dtor */
This is a little awkward, IMO. page.index is actually pgoff_t, not
unsigned long. Conceivably someday someone may want to make pgoff_t 64-bit
on 32-bit machines, for example. Yes, that'll still work, but it's still
awkward.
Is it not possible to put the dtor address into some address-type field
within the pageframe rather than into an integer-type?
Or just leave it using page.mapping? Anyone who uses page.mapping of a
tail page in a compound page should go oops.
> +/*
> + * Ignore PageCompound when checking page_count for debugging -
> + * page_private might be corrupt; but never expose this to wider use.
> + */
> +static inline int __page_count(struct page *page)
> +{
> + return atomic_read(&page->_count) + 1;
> +}
hm, spose so. Nick has a patch which unskews page.count which will need
updating for this.
<checks>
Looks like I didn't apply Nick's patch yet - mm/ in -mm has a nutty amount
of stuff in it now.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
