On Fri, Nov 18, 2022 at 2:12 AM Hugh Dickins <hughd@xxxxxxxxxx> wrote:
...
> @@ -1308,31 +1285,29 @@ void page_add_anon_rmap(struct page *page,
>
> if (unlikely(PageKsm(page)))
> lock_page_memcg(page);
> - else
> - VM_BUG_ON_PAGE(!PageLocked(page), page);
>
> - if (likely(!PageCompound(page))) {
> + if (likely(!compound /* page is mapped by PTE */)) {
> first = atomic_inc_and_test(&page->_mapcount);
> nr = first;
> + if (first && PageCompound(page)) {
> + struct page *head = compound_head(page);
> +
> + lock_compound_mapcounts(head, &mapcounts);
> + mapcounts.subpages_mapcount++;
> + nr = !mapcounts.compound_mapcount;
> + unlock_compound_mapcounts(head, &mapcounts);
> + }
> + } else if (PageTransHuge(page)) {
> + /* That test is redundant: it's for safety or to optimize out */
>
> - } else if (compound && PageTransHuge(page)) {
> lock_compound_mapcounts(page, &mapcounts);
> first = !mapcounts.compound_mapcount;
> mapcounts.compound_mapcount++;
> if (first) {
> - nr = nr_pmdmapped = thp_nr_pages(page);
> - if (mapcounts.subpages_mapcount)
> - nr = nr_subpages_unmapped(page, nr_pmdmapped);
> + nr_pmdmapped = thp_nr_pages(page);
> + nr = nr_pmdmapped - mapcounts.subpages_mapcount;
> }
> unlock_compound_mapcounts(page, &mapcounts);
> - } else {
> - struct page *head = compound_head(page);
> -
> - lock_compound_mapcounts(head, &mapcounts);
> - mapcounts.subpages_mapcount++;
> - first = subpage_mapcount_inc(page);
> - nr = first && !mapcounts.compound_mapcount;
> - unlock_compound_mapcounts(head, &mapcounts);
> }
>
> VM_BUG_ON_PAGE(!first && (flags & RMAP_EXCLUSIVE), page);
Hi Hugh, I got the following warning from the removed "else" branch.
Is it legit? Thanks.
mm/rmap.c:1236:13: warning: variable 'first' is used uninitialized
whenever 'if' condition is false [-Wsometimes-uninitialized]
} else if (PageTransHuge(page)) {
^~~~~~~~~~~~~~~~~~~
mm/rmap.c:1248:18: note: uninitialized use occurs here
VM_BUG_ON_PAGE(!first && (flags & RMAP_EXCLUSIVE), page);
^~~~~
