Messages in this thread | | | Date | Tue, 22 Nov 2022 01:03:42 -0800 (PST) | From | Hugh Dickins <> | Subject | Re: [PATCH 1/3] mm,thp,rmap: subpages_mapcount of PTE-mapped subpages |
| |
On Mon, 21 Nov 2022, Kirill A. Shutemov wrote: > On Fri, Nov 18, 2022 at 01:12:03AM -0800, Hugh Dickins wrote: > > Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Thanks a lot for all these, Kirill.
> > Few minor nitpicks below. > ... > > static inline void page_dup_file_rmap(struct page *page, bool compound) > > { > > - if (PageCompound(page)) > > - page_dup_compound_rmap(page, compound); > > - else > > + if (likely(!compound /* page is mapped by PTE */)) > > I'm not a fan of this kind of comments. > > Maybe move above the line (here and below)?
Okay, done throughout. I wouldn't have added those comments, but Linus had another "simmering hatred", of the "compound" arguments: he found them very confusing.
The real fix is to rename them, probably to pmd_mapped; or better, pass down an int nr_pages as he suggested; but I'm wary of the HPAGE_NR_PAGES build bug, and it wants to be propagated through various other files (headers and mlock.c, maybe more) - not a cleanup to get into now.
> > > atomic_inc(&page->_mapcount); > > + else > > + page_dup_compound_rmap(page); > > } ... > > @@ -1176,20 +1157,16 @@ void page_dup_compound_rmap(struct page *page, bool compound) > > * Note that hugetlb does not call page_add_file_rmap(): > > * here is where hugetlb shared page mapcount is raised. > > */ > > - if (PageHuge(page)) { > > - atomic_inc(compound_mapcount_ptr(page)); > > - return; > > - } > > + if (PageHuge(head)) { > > + atomic_inc(compound_mapcount_ptr(head)); > > > > Remove the newline?
It was intentional there, I thought it was easier to read that way; but since this gets reverted in the next patch, I've no reason to fight over it - removed.
Hugh
| |