On Wed, Oct 14, 2020 at 09:12:16AM -0700, Darrick J. Wong wrote:
> On Wed, Oct 14, 2020 at 04:03:44AM +0100, Matthew Wilcox (Oracle) wrote:
> > We may get tail pages returned from vfs_dedupe_get_page(). If we do,
> > we have to call page_mapping() instead of dereferencing page->mapping
> > directly. We may also deadlock trying to lock the page twice if they're
> > subpages of the same THP, so compare the head pages instead.
> > static void vfs_lock_two_pages(struct page *page1, struct page *page2)
> > {
> > + page1 = thp_head(page1);
> > + page2 = thp_head(page2);
>
> Hmm, is this usage (calling thp_head() to extract the head page from an
> arbitrary page reference) a common enough idiom that it doesn't need a
> comment saying why we need the head page?
It's pretty common. Lots of times it gets hidden inside macros,
and sometimes it gets spelled as 'compound_head' instead of
thp_head. The advantage of thp_head() is that it compiles away if
CONFIG_TRANSPARENT_HUGEPAGE is disabled, while compound pages always
exist.
> I'm asking that genuinely-- thp_head() is new to me but maybe it's super
> obvious to everyone else? Or at least the mm developers? I suspect
> that might be the case....?
thp_head is indeed new. It was merged in August this year, partly in
response to Dave Chinner getting annoyed at the mixing of metaphors --
some things were thp_*, some were hpage_* and some were compound_*.
Now everything is in the thp_* namespace if it refers to THPs.
> Also, I was sort of thinking about sending a patch to Linus at the end
> of the merge window moving all the remap/clone/dedupe common code to a
> separate file to declutter fs/read_write.c and mm/filemap.c. Does that
> sound ok?
I don't think that would bother me at all.
