From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <error27@gmail.com>
Date: Tue, 29 Nov 2022 07:57:15 +0300
From: Dan Carpenter <error27@gmail.com>
Subject: Re: hugetlb BUILD REGRESSION in linux-next20221121
Message-ID: <Y4WRK97aXPjqlWnV@kadam>
References: <c009bf64-9d7c-b831-9087-936a3e0801d4@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <c009bf64-9d7c-b831-9087-936a3e0801d4@oracle.com>
To: Sidhartha Kumar <sidhartha.kumar@oracle.com>
Cc: smatch@vger.kernel.org
List-ID: <smatch.vger.kernel.org>

On Mon, Nov 28, 2022 at 02:34:54PM -0800, Sidhartha Kumar wrote:
> Hello,
>=20
> One of my patches in linux-next was flagged as a Unverified Error/Warning
> with the following warning[1]:
>=20
> mm/hugetlb.c:2073 alloc_pool_huge_page() error: uninitialized symbol
> 'folio'.
>=20
> The relevant change is:
>=20
> -=A0=A0=A0 struct page *page;
> +=A0=A0=A0 struct folio *folio;
> =A0=A0=A0=A0 int nr_nodes, node;
> =A0=A0=A0=A0 gfp_t gfp_mask =3D htlb_alloc_mask(h) | __GFP_THISNODE;
>=20
> =A0=A0=A0=A0 for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed=
) {
> -=A0=A0=A0 =A0=A0=A0 page =3D alloc_fresh_huge_page(h, gfp_mask, node, no=
des_allowed,
> -=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 =A0=A0=A0 =A0=A0=A0 =A0=A0=A0 node_alloc_n=
oretry);
> -=A0=A0=A0 =A0=A0=A0 if (page)
> +=A0=A0=A0 =A0=A0=A0 folio =3D alloc_fresh_hugetlb_folio(h, gfp_mask, nod=
e,
> +=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 =A0=A0=A0 =A0=A0=A0 nodes_allowed, node_al=
loc_noretry);
> +=A0=A0=A0 =A0=A0=A0 if (folio)
> =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 break;
> =A0=A0=A0=A0 }
>=20
> -=A0=A0=A0 if (!page)
> +=A0=A0=A0 if (!folio)
>=20
> It looks like I can initialize folio to NULL to avoid this error but I'm =
not
> sure how this would cause a regression as previously the page variable was
> unitialized as well. Please let me know if I am missing something in my
> patch or if this should be ignored.

Both the original and the new code trigger a Smatch warning.  I don't
know why the kbuild-bot marks this as a new warning.  Possibly that's
because the variable name has changed?  The kbuild-bot is run by Intel.

The problem is obviously that Smatch doesn't know that we always enter
the loop.  There are hack arounds that I could do for this, but
sometimes we dont' actually enter the loop so changing this will hide
bugs...  I am conflicted on this.

regards,
dan carpenter