From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E379AC433DB for ; Mon, 25 Jan 2021 18:27:24 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 65F3722DD6 for ; Mon, 25 Jan 2021 18:27:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 65F3722DD6 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C1BAA6B0033; Mon, 25 Jan 2021 13:27:23 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BF41C6B005D; Mon, 25 Jan 2021 13:27:23 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B31826B006C; Mon, 25 Jan 2021 13:27:23 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0217.hostedemail.com [216.40.44.217]) by kanga.kvack.org (Postfix) with ESMTP id 9B8B06B0033 for ; Mon, 25 Jan 2021 13:27:23 -0500 (EST) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 5A3F23628 for ; Mon, 25 Jan 2021 18:27:23 +0000 (UTC) X-FDA: 77745129966.06.wind42_280e9a727587 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin06.hostedemail.com (Postfix) with ESMTP id 35B101004165F for ; Mon, 25 Jan 2021 18:27:23 +0000 (UTC) X-HE-Tag: wind42_280e9a727587 X-Filterd-Recvd-Size: 8464 Received: from mail-pj1-f45.google.com (mail-pj1-f45.google.com [209.85.216.45]) by imf34.hostedemail.com (Postfix) with ESMTP for ; Mon, 25 Jan 2021 18:27:22 +0000 (UTC) Received: by mail-pj1-f45.google.com with SMTP id gx1so138109pjb.1 for ; Mon, 25 Jan 2021 10:27:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=rH51Ivc55ptvFbtJscFbdlO/ZDzsRHEdg+dEPlaRK4c=; b=KqBWErOVNKTWOR2DkBApBRlPLMxnS/H5zTdCCjP0/wpyrm11wzzILf4M1sxaOspxuK 5cVwoCqSLEshDs31dHsdSNgNgiAxT8aYJDbSRUjVbuNKXsb9P+zoUZxnz0YPL1BqBl00 xBkkgg73CdrVFG9txay538Ptv6lhPUd4oIetXRpXdnxyuvwL0zmEZ8aWsuszeSIyWI65 6bikn3vme9aV8MHnCbMSb52yqj5wga234PPOjlMHPR5URXix8xoe4cfZmbaPUkdF9QTE LEMvEvBlTKlcHyxE9A/9SfMGn9uF+0Ldx8pXQBSzby8XGbfriXO02V+db8IZ9TWaQy/7 hwhA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rH51Ivc55ptvFbtJscFbdlO/ZDzsRHEdg+dEPlaRK4c=; b=dj3yQtZRK5lTh6NB2ZvpZ3p2yhJgxXqKXBgrFj+ciwnqTAH7bpLtGNlYrB1Du1SUcv LjhiHgSZU3xf5H5FEWnrPrNYqom9NRpNQaf9nfHjVoYvaNn65n3AmRk904JQbKpCErLT QFjKltyGWYYP36E0biFJ5CEtQ/yZ9S7uwQ11I107gjTyf+80PCM0rVnCRuOPOQAk/lK/ ajohuq5WmiSJhv/J9ORsiZCJR/sygS8+SVnq8wpZVt182vhrs2vhIY0RcHIIZ4RqemnB Sn//oRrpqt987V97Qhr7dQvwJEXt0CkSmCwzx0fqo8zbMIsAEvVqDPCJeWCPSukGtknp rLuQ== X-Gm-Message-State: AOAM532Z3B9t5cwSo0qtQhlYJovhnSjNtwZeqTJLRYGxwcxyf3yKIDto zAi4LXLBXkeRfCW+5j1ELZIx4Xjx/k18lhKmimTja5X7+3eYyQ== X-Google-Smtp-Source: ABdhPJx2YOu9DODyhHPIBO8CoLu8MRNSftF+KFP3I6bHrp5wBDrDHzYq5B2GpE8c6EuB99CWrH6Sm4Mtc6l9IDAygcs= X-Received: by 2002:a17:90b:228f:: with SMTP id kx15mr1617388pjb.32.1611599241506; Mon, 25 Jan 2021 10:27:21 -0800 (PST) MIME-Version: 1.0 References: <20201231212852.3175381-1-dennis@kernel.org> <20210104234651.GA3548546@ubuntu-m3-large-x86> In-Reply-To: From: Nick Desaulniers Date: Mon, 25 Jan 2021 10:27:11 -0800 Message-ID: Subject: Re: [PATCH] percpu: fix clang modpost warning in pcpu_build_alloc_info() To: Arnd Bergmann Cc: Dennis Zhou , Nathan Chancellor , Tejun Heo , Christoph Lameter , Linux-MM , "linux-kernel@vger.kernel.org" , clang-built-linux , kbuild-all@lists.01.org, kernel test robot Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Jan 25, 2021 at 3:07 AM Arnd Bergmann wrote: > > On Tue, Jan 5, 2021 at 1:55 AM Dennis Zhou wrote: > > > > On Mon, Jan 04, 2021 at 04:46:51PM -0700, Nathan Chancellor wrote: > > > On Thu, Dec 31, 2020 at 09:28:52PM +0000, Dennis Zhou wrote: > > > > > > > > Hi Nathan, > > > > > > > > Hi Dennis, > > > > > > I did a bisect of the problematic config against defconfig and it points > > > out that CONFIG_GCOV_PROFILE_ALL is in the bad config but not the good > > > config, which makes some sense as that will mess with clang's inlining > > > heuristics. It does not appear to be the single config that makes a > > > difference but it gives some clarity. > > > > > > > Ah, thanks. To me it's kind of a corner case that I don't have a lot of > > insight into. __init code is pretty limited and this warning is really > > at the compilers whim. However, in this case only clang throws this > > warning. > > > > > I do not personally have any strong opinions around the patch but is it > > > really that much wasted memory to just annotate mask with __refdata? > > > > It's really not much memory, 1 bit per max # of cpus. The reported > > config is on the extreme side compiling with 8k NR_CPUS, so 1kb. I'm > > just not in love with the idea of adding a patch to improve readability > > and it cost idle memory to resolve a compile time warning. > > > > If no one else chimes in in the next few days, I'll probably just apply > > it and go from there. If another issue comes up I'll drop this and tag > > it as __refdata. > > I've come across this one again in linux-next today, and found that > I had an old patch for it already, that I had never submitted: > > From 7d6f40414490092b86f1a64d8c42426ee350da1a Mon Sep 17 00:00:00 2001 > From: Arnd Bergmann > Date: Mon, 7 Dec 2020 23:24:20 +0100 > Subject: [PATCH] mm: percpu: fix section mismatch warning > > Building with arm64 clang sometimes (fairly rarely) shows a > warning about the pcpu_build_alloc_info() function: > > WARNING: modpost: vmlinux.o(.text+0x21697c): Section mismatch in > reference from the function cpumask_clear_cpu() to the variable > .init.data:pcpu_build_alloc_info.mask > The function cpumask_clear_cpu() references > the variable __initdata pcpu_build_alloc_info.mask. > This is often because cpumask_clear_cpu lacks a __initdata > annotation or the annotation of pcpu_build_alloc_info.mask is wrong. > > What appears to be going on here is that the compiler decides to not > inline the cpumask_clear_cpu() function that is marked 'inline' but not > 'always_inline', and it then produces a specialized version of it that > references the static mask unconditionally as an optimization. > > Marking cpumask_clear_cpu() as __always_inline would fix it, as would > removing the __initdata annotation on the variable. I went for marking > the function as __attribute__((flatten)) instead because all functions I had to look this one up; it's new to me! https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes https://awesomekling.github.io/Smarter-C++-inlining-with-attribute-flatten/ Seems pretty cool/flexible to control inlining on the caller side! At the least though, we should avoid open coding the function attributes. See include/linux/compiler_attributes.h Testing quickly in godbolt, __flatten__ has been supported since at least clang 3.5 and gcc 4.4, FWIW (so it doesn't need a __has_attribute guard). > called from it are really meant to be inlined here, and it prevents > the same problem happening here again. This is unlikely to be a problem > elsewhere because there are very few function-local static __initdata > variables in the kernel. > > Fixes: 6c207504ae79 ("percpu: reduce the number of cpu distance comparisons") > Signed-off-by: Arnd Bergmann > > diff --git a/mm/percpu.c b/mm/percpu.c > index 5ede8dd407d5..527181c46b08 100644 > --- a/mm/percpu.c > +++ b/mm/percpu.c > @@ -2662,10 +2662,9 @@ early_param("percpu_alloc", percpu_alloc_setup); > * On success, pointer to the new allocation_info is returned. On > * failure, ERR_PTR value is returned. > */ > -static struct pcpu_alloc_info * __init pcpu_build_alloc_info( > - size_t reserved_size, size_t dyn_size, > - size_t atom_size, > - pcpu_fc_cpu_distance_fn_t cpu_distance_fn) > +static struct pcpu_alloc_info * __init __attribute__((flatten)) > +pcpu_build_alloc_info(size_t reserved_size, size_t dyn_size, size_t atom_size, > + pcpu_fc_cpu_distance_fn_t cpu_distance_fn) > { > static int group_map[NR_CPUS] __initdata; > static int group_cnt[NR_CPUS] __initdata; > > > Not sure if this would be any better than your patch. > > Arnd > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAK8P3a2ZWfNeXKSm8K_SUhhwkor17jFo3xApLXjzfPqX0eUDUA%40mail.gmail.com. -- Thanks, ~Nick Desaulniers