Re: [PATCH 00/16] Sanitize usage of ->flags and ->mapping for tail pages

[Hugh Dickins]

On Thu, 19 Mar 2015, Kirill A. Shutemov wrote:

> Currently we take naive approach to page flags on compound -- we set the
> flag on the page without consideration if the flag makes sense for tail
> page or for compound page in general. This patchset try to sort this out
> by defining per-flag policy on what need to be done if page-flag helper
> operate on compound page.
> 
> The last patch in patchset also sanitize usege of page->mapping for tail
> pages. We don't define meaning of page->mapping for tail pages. Currently
> it's always NULL, which can be inconsistent with head page and potentially
> lead to problems.
> 
> For now I catched one case of illigal usage of page flags or ->mapping:
> sound subsystem allocates pages with __GFP_COMP and maps them with PTEs.
> It leads to setting dirty bit on tail pages and access to tail_page's
> ->mapping. I don't see any bad behaviour caused by this, but worth fixing
> anyway.

But there's nothing to fix there.  We're more used to having page->mapping
set by filesystems, but it is normal for drivers to have pages with NULL
page->mapping mapped into userspace (and it's not accidental that they
appear !PageAnon); and subpages of compound pages mapped into userspace,
and set_page_dirty applied to them.

> 
> This patchset makes more sense if you take my THP refcounting into
> account: we will see more compound pages mapped with PTEs and we need to
> define behaviour of flags on compound pages to avoid bugs.

Yes, I quite understand that you want to clarify the usage of different
page flags to yourself, to help towards a policy of what to do with each
of them when subpages of a huge compound page are mapped into userspace;
but I don't see that we need this patchset in the kernel now, given that
it adds unnecessary overhead into several low-level inline functions.

I'm surprised that Andrew has fast-tracked it into his mmotm tree:
I don't think it's harmful beyond the overhead, but it seems premature:
let's wait until we get some benefit too?

Hugh
--
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/