From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=message-id : date : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=corp-2022-7-12; bh=EwQ0S7EEU5nh553wkssg4yA2bjQpY5k3PzDWvDUF0Pw=; b=D1WeGLBDB4dUP7YIh7V9XA+//m5aEsQc93FA16bDdE4CfqRCC4NejVPksqZigfvBYmvg fLL+1UZWpsOeo5Uds1NtyOBHOssdWjvm+Xs9lSGHHg0RvQOdh0U9wmPZpQtuJaHncE4K VEr+NiOmv/Bjnib6CqzCUWNa+VxV1NeA3idCxoddD9BDDTVaQT+Az2KIIetIAFT3RfFC R0lTQTBdTdGwRtH0nMXBWg2rg14m3XMbnPh1Z+1y+eZcLMgQnKAjWewvpDYmnDAt4EoA A15yIIHYYNVtO/52mT6YEdbvDeVoP6Lw1QPmGS++2Ku3QHTZob/+ZyZPWMESijPFp7jg hQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.onmicrosoft.com; s=selector2-oracle-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=EwQ0S7EEU5nh553wkssg4yA2bjQpY5k3PzDWvDUF0Pw=; b=pKPe8qgvF342z7mqpfyC1AbV7wdxNcFRudsY759MC7WXqKuLkN/OCTvPd1LaLHn/d8LyacaPBHvgBRDCtr2ELUwah+Ku6CRppkaNRDo4RFeEmK4RN87pCZ50tdGXP0mxdNEI1JpCGtZ3A/KO+0sNB8Wkmpv7m1jyOiiBKIlJkis= Message-ID: <4f20611e-2d4e-0bac-3620-7853dff93506@oracle.com> Date: Tue, 29 Nov 2022 09:23:57 -0800 Subject: Re: hugetlb BUILD REGRESSION in linux-next20221121 References: <44e653d1-32ec-e6ab-7c48-baf8d406ee31@rasmusvillemoes.dk> Content-Language: en-US From: Sidhartha Kumar In-Reply-To: <44e653d1-32ec-e6ab-7c48-baf8d406ee31@rasmusvillemoes.dk> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit MIME-Version: 1.0 To: Rasmus Villemoes , Dan Carpenter Cc: smatch@vger.kernel.org List-ID: On 11/28/22 11:40 PM, Rasmus Villemoes wrote: > On 29/11/2022 05.57, Dan Carpenter wrote: >> On Mon, Nov 28, 2022 at 02:34:54PM -0800, Sidhartha Kumar wrote: >>> Hello, >>> >>> One of my patches in linux-next was flagged as a Unverified Error/Warning >>> with the following warning[1]: >>> >>> mm/hugetlb.c:2073 alloc_pool_huge_page() error: uninitialized symbol >>> 'folio'. >>> >>> The relevant change is: >>> >>> -    struct page *page; >>> +    struct folio *folio; >>>      int nr_nodes, node; >>>      gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE; >>> >>>      for_each_node_mask_to_alloc(h, nr_nodes, node, nodes_allowed) { >>> -        page = alloc_fresh_huge_page(h, gfp_mask, node, nodes_allowed, >>> -                        node_alloc_noretry); >>> -        if (page) >>> +        folio = alloc_fresh_hugetlb_folio(h, gfp_mask, node, >>> +                    nodes_allowed, node_alloc_noretry); >>> +        if (folio) >>>              break; >>>      } >>> >>> -    if (!page) >>> +    if (!folio) >>> >>> 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. > It seems that a reasonably clean way to deal with this, since there's > not a lot of code following the loop, is to simply move that code into > the "if (page/folio)" branch. I.e. change the tail to > > > if (page) { > free_huge_page(page); /* free it into the hugepage allocator */ > return 1; > } > } // this is the end of loop > return 0; > > or however it looks after the above change. Thanks for the suggestion, this looks like a good way to avoid the smatch warning and I'll add it to the next version of my patch. Thanks. Sidhartha Kumar > Rasmus >