Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 18 Jan 2023 09:07:50 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Linus Torvalds <torvalds@linux-foundation.org>
CC:     Vlastimil Babka <vbabka@suse.cz>,
        "Sang, Oliver" <oliver.sang@intel.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        "oe-lkp@lists.linux.dev" <oe-lkp@lists.linux.dev>,
        lkp <lkp@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Jann Horn <jannh@google.com>,
        "Song, Youquan" <youquan.song@intel.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Jan Kara <jack@suse.cz>, John Hubbard <jhubbard@nvidia.com>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Matthew Wilcox <willy@infradead.org>,
        Michal Hocko <mhocko@kernel.org>,
        Muchun Song <songmuchun@bytedance.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        "Yin, Fengwei" <fengwei.yin@intel.com>, <hongjiu.lu@intel.com>
Subject: Re: [linus:master] [hugetlb] 7118fc2906:
 kernel_BUG_at_lib/list_debug.c
Message-ID: <Y8dGZkhytk25iyr1@feng-clx>
References: <202301170941.49728982-oliver.sang@intel.com>
 <2f483247-da76-9ec9-3e51-f690939f4585@suse.cz>
 <Y8ZVxJSZdtEk8Nco@feng-clx>
 <Y8aSc5xGO+rW2pyo@feng-clx>
 <CAHk-=wiS84nS9apjs_Vt=jjZ_+j+F8HQ3B+ABSvbzcqtW9x5Kg@mail.gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <CAHk-=wiS84nS9apjs_Vt=jjZ_+j+F8HQ3B+ABSvbzcqtW9x5Kg@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?90Lr60P8X6WFWLwtpMCqhQrK7F2qCOYDHdDT+ysV1QaBNgUxigmtykUBCDv6?=
 =?us-ascii?Q?1qZpbaXU3l5OBP1MeLdev2lRBwwIcUCgObPJqQqOMcrSD1ox4NQwFMCVb3HM?=
 =?us-ascii?Q?/y28OLHYnqf/PAZoT6tZaRdwmF0W0tjip/a8k9dwKGK97njaWEcRDdaKQlRF?=
 =?us-ascii?Q?HHri8xHZparqfiF6bfwkUtTgXknSVz6T9fQlBRh1sRSUs052PIeat+0/qfwa?=
 =?us-ascii?Q?ynvTZylmDoDcu5lPwoRI3CgJlfuDAQ1AIJQk+zuH3B/TjiO7lOc6usZb8v4D?=
 =?us-ascii?Q?mDxfJ1avhG1Zo+ZlUijdfDoH4ASJKdpLHq5XDNOuGZhAi63gcnb3tZxiPP4C?=
 =?us-ascii?Q?jUdSr2Alrj9hssGv8cbcsEQCL7VZYdXqUZcUxEMJ9bWFN+SmIAaTX7GqNWxT?=
 =?us-ascii?Q?4Gwmd4TMExUq2VsIkT60xjQRVICgqy+uOkETaafi3i3/nvIel74tGdiz+p2t?=
 =?us-ascii?Q?0miT1bzR7gqTUq+lNQNm0uuvZ3Ow4TZQfPftPTlvR9X/i6PM6Wr4McbLyhG0?=
 =?us-ascii?Q?iR9ShmnZx5fQzoGa+gFL5nOa/F6ppRzID34x47T9pjpBZ3IQVW6nOqcu/X36?=
 =?us-ascii?Q?ubrl5KMe9nGl6buFVf9OT0htGv1h2FNBtYewFp/JCyV/HjVREI3rz2HT1Vay?=
 =?us-ascii?Q?ExK7dd9cWUwFNwwJJU99Hb9KEzQfnBTBXF24gOwO24oPu4fnfCb3puzGKykB?=
 =?us-ascii?Q?Z6Y7ph9TPmNK2hHkJQPHELKeW3o3ASfvpRcBnvq9yH4C0wPbLV6jeyqWs2BX?=
 =?us-ascii?Q?TyoSGPNEoUtyTGSLxu+NztlJ5ZKJWhgq9iZtx94jBPV2bLrnxE7QJ/BVC3Yt?=
 =?us-ascii?Q?vcCP6Oni1Rs8c8JTwtfZMNDcoIWBrxWupJGv4Ja/mBnqIw7hhzCYSYt3Vwgw?=
 =?us-ascii?Q?hHaMm/laNl0yL5Z80AEUFnC0y57oQuo/dlUjvcwWmrtQs5qrx0ID4qbPhE4h?=
 =?us-ascii?Q?U3wo/D936u+CqpJUBvoc2ltOOsr5nG5txX3zvS2GcbUUmsh/V6mI4vC7n8mp?=
 =?us-ascii?Q?i/sr/mctsLu1doCmvWEpfNQodQHXUA6EZ3x/ZDTHDv8ZOgTLuzWd1q+I1ojm?=
 =?us-ascii?Q?TfYj1ZswxXsjG1ZyMjPRtOrLWCEP4fi+SiV1+cZc1q/EwO9DF8ANUWGoXgfv?=
 =?us-ascii?Q?g5punRvnyc4gpqxP+ADffeeV46BzJg5R6oxkZ8PmJMnG1skFx6gMMbU+RJ5K?=
 =?us-ascii?Q?yz8Y9lM/rtE+TIMkAaZEZ83Anc49kVoxCHBAoB8uFKbZBwZmqfInv9ws6O5E?=
 =?us-ascii?Q?mlBmdJxbnHN0FwnwEohEXdTq/dX82QTbfO4/6PnfjmQClHsbaQB9RgZ+uma/?=
 =?us-ascii?Q?G1MQW61CVP8OLYyzmzoXbdvUPTWRjUewYx9V6nACc+RCIyFDgMcosnAfucr2?=
 =?us-ascii?Q?DULXIqaZmCnBvMRp/1eSyDsRzmeE6+gfyv49fflS3FoFn9MDsBSbkoR3Xkg/?=
 =?us-ascii?Q?fnm2YtU9d5mppOruHXyRHeMRmC2YXJZ9+bVreEW0zlXuHV8BJ/diDVh9RAMK?=
 =?us-ascii?Q?pBPrs0v1fdRJxNG/SoOXaoAc2qd+0dsH73nYBRxCqB6booBvKdJUmnwf3bfv?=
 =?us-ascii?Q?E7zuoe2WCdZejGNH6DabVniiDAUNtIt65T1QU9vp?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 85cc8bc3-84ec-409e-5c9a-08daf8f0cce0
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jan 2023 01:10:34.1941
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: aOrSALsTCZZpT1Ns3CxZnIFNIpTyzP7Oeovky6EmwYWOZMkdbIZpkHRodOZQzzDdoceCW3QxXjeuR7GN5cIFUg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB6913
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,LOTS_OF_MONEY,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jan 17, 2023 at 10:25:17AM -0800, Linus Torvalds wrote:
> On Tue, Jan 17, 2023 at 4:22 AM Feng Tang <feng.tang@intel.com> wrote:
> >
> > With the following patch to use 'O1' instead 'O2' gcc optoin for
> > page_alloc.c, the list corruption issue can't be reproduced for
> > commit 7118fc2906 in 1000 runs.
> 
> Ugh.
> 
> It would be lovely if you could just narrow it down with
> 
>   #pragma GCC optimize ("O1")
>  ...
>   #pragma GCC optimize ("O2")
> 
> around just that prep_compound_page(), but when I tried it myself I
> get some function attribute mismatch errors.
 
Good to know, thanks! will try this.

> 
> > As is can't be reproduced with X86_64 build, it could be i386
> > compiling related.
> 
> Your particular config causes a huge amount of nasty 64-bit arithmetic
> according to the objdump code, with sequences like
> 
>   c13b3cbb:       83 05 d0 28 6c c5 01    addl   $0x1,0xc56c28d0
>   c13b3cc2:       83 15 d4 28 6c c5 00    adcl   $0x0,0xc56c28d4
> 
> which seems to be just from some coverage profiling being on
> (CONFIG_GCOV?), or something. It makes it very hard to read the code.
> 
> You also have UBSAN enabled, which - again - makes for some really
> grotty asm that hides any actual logic.

Yes, you are right, CONFIG_GCOV_PROFILE_ALL and a bunch of UBSAN
configs are enabled.

AFAIK, this is a random generated config for sanity test, Oliver/Philip
can correct me on this.

> Finally, your objdump version also does some horrendous decoding, like
> 
>   c13b3e29:       8d b4 26 00 00 00 00    lea    0x0(%esi,%eiz,1),%esi
> 
> which is just a 7-byte 'nop' instruction, but again, it makes it
> rather hard to actually read the code.
> 
> With the i386 defconfig, gcc generates a function that is just ~30
> instructions for me, so this makes a huge difference in the legibility
> of the code.
> 
> I wonder if you can recreate the issue with a much more
> straightforward config. By all means, leave DEBUG_PAGEALLOC and SLUB
> debugging on, but without the things like UBSAN and GCOV.

Oliver and I can try this, and will report back.

> > I also objdumped 'prep_compound_page' for vmlinux of 7118fc2906 and
> > its parent commit 48b8d744ea84, which have big difference than the
> > simple 'set_page_count()' change, but I can't tell which part is
> > abnormal, so attach them for further check.
> 
> Yeah, I can't make heads or tails of them either, see above on how
> illegible the objdump files are. And that's despite not even having
> all of prep_compound_page() in them (it's missing
> prep_compound_page.cold, which is probably just UBSAN fixup code, but
> who knows..)
> 
> That said, with the i386 defconfig, the only change from adding
> set_page_count() to the loop seems to be exactly that:
> 
>  .L589:
> -       movl    $1024, 12(%eax)
> +       movl    $0, 28(%eax)
>         addl    $32, %eax
> +       movl    $1024, -20(%eax)
>         movl    %esi, -28(%eax)
>         movl    $0, -12(%eax)
>         cmpl    %edx, %eax
> 
> (don't ask me why gcc does *one* access using the pre-incremented
> pointer, and then the rest to the post-incremented ones, but whatever
> - it means that it's not just "add a mov $0", it's also changing how
> the
> 
>         p->mapping = TAIL_MAPPING;
> 
> instruction is done, which is that
> 
> -       movl    $1024, 12(%eax)
> +       movl    $1024, -20(%eax)
> 
> part of the change)

This version of assembly diff looks more reasonable. One thing I
note is the sizeof(page) with this config is 32, while it's 40 for
the randconfig used in this report.

Thanks for the analyzing and debug suggestion!

- Feng

>              Linus