Re: [PATCH 1/3] mm,thp,rmap: subpages_mapcount of PTE-mapped subpages

[Hugh Dickins]

On Fri, 18 Nov 2022, Yu Zhao wrote:
> 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,
...
> >
> >         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);
>                         ^~~~~

Thanks a lot for that.  From the compiler's point of view, it is
certainly a legit warning.  From our point of view, it's unimportant,
because we know that page_add_anon_rmap() should only ever be called
with compound true when PageTransHuge(page) (and should never be
called with compound true when TRANSPARENT_HUGEPAGE is disabled):
so it's a "system error" if first is uninitialized there.

But none of us want a compiler warning: I'll follow up with a fix
patch, when I've decided whether it's better initialized to true
or to false in the impossible case...

Although the same pattern is used in other functions, this is the
only one of them which goes on to use "first" or "last" afterwards.

Hugh