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=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 66341C433DB for ; Mon, 4 Jan 2021 23:46:56 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CE29E207B0 for ; Mon, 4 Jan 2021 23:46:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CE29E207B0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 386128D003B; Mon, 4 Jan 2021 18:46:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 336D88D0036; Mon, 4 Jan 2021 18:46:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 225A78D003B; Mon, 4 Jan 2021 18:46:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0034.hostedemail.com [216.40.44.34]) by kanga.kvack.org (Postfix) with ESMTP id 0D3EC8D0036 for ; Mon, 4 Jan 2021 18:46:55 -0500 (EST) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id C8D8D3629 for ; Mon, 4 Jan 2021 23:46:54 +0000 (UTC) X-FDA: 77669730348.25.lock03_5215673274d4 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin25.hostedemail.com (Postfix) with ESMTP id A35CC1804E3A8 for ; Mon, 4 Jan 2021 23:46:54 +0000 (UTC) X-HE-Tag: lock03_5215673274d4 X-Filterd-Recvd-Size: 7324 Received: from mail-qt1-f179.google.com (mail-qt1-f179.google.com [209.85.160.179]) by imf14.hostedemail.com (Postfix) with ESMTP for ; Mon, 4 Jan 2021 23:46:53 +0000 (UTC) Received: by mail-qt1-f179.google.com with SMTP id u21so19747200qtw.11 for ; Mon, 04 Jan 2021 15:46:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Titf8x1fnSDltA5xxQs29GH+bpNslU3vFYKElIQuG6w=; b=UhUadZD8lOMwa+Sed5RMf2DZTdRKBT+RmtE1cnmlb62Mriz/FGKLSKYErHQpwAxFQJ mnI2RuV6HLKzuDk+m1Xxw+zALXui1RupcIKAK5XxwPzelwxyCo4eqp2H3wfaKXM5Kr58 V+LcSQ7vmCT+YPyKo4v3f7lxolxxs4TrCR+Y59fdeBcb4KPJ0iqiRmN1Sf/JMJqg+0Gy 6YlGzw0zLQhnHSfFdMx/NGqE/RNHpI0GkYmzk9qX11ELglQmITltuVAqMRZVSTtad7za wXsAgfO5H9GrYeH8MtUP/RZOGGVA8fpYBYxz7FuyBrUR5rxIjakjeewLdNx9k4Dh8GPe GW3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Titf8x1fnSDltA5xxQs29GH+bpNslU3vFYKElIQuG6w=; b=c/c3nkS295HXtlU4nsNi9Or0PomdWDSqAFWfm6qjyuBFJH4f39wa7fqkiMoKCSL6mI 9snAHvej7PfDwHhNHCDToXLbeZL6NWt3PxWzWy+2Ydz4SrUjLUheByTkJs1vTv2gxcTG 9abEnZD8g5B/Vm5SYBIqcz+3Lv/wuTvUgAYvCBLxf7n6zGFISdyppuW9YpgwkaA7wj3R jaK+V+M37D66d9cjittaDq09OLsw6j5wbIaMGh1lLOeYfJF/87sZewRouV1DxY8LOj+P RlCfn3iptkbbPWFZy2OnmvDpD4lv4IeSBNgwWleuedLHMCXUuI8spN8Tum1GgnfO0Rf7 V/Ig== X-Gm-Message-State: AOAM530+/2FQxfk0Fc0pdZlirqIgw5bcRDuY0RAVOaX27dCOiMGqTHnH wU1PBVK1nxxz1Ym1O5gXFJg= X-Google-Smtp-Source: ABdhPJx12uFWI2THN0aVECziLp2DMRNGSOgWv6NMPBIpeCirGPZXBzkoXxqOekXbRwkLEp1YgiE+Vw== X-Received: by 2002:aed:2e47:: with SMTP id j65mr74442331qtd.231.1609804012936; Mon, 04 Jan 2021 15:46:52 -0800 (PST) Received: from ubuntu-m3-large-x86 ([2604:1380:45f1:1d00::1]) by smtp.gmail.com with ESMTPSA id 195sm38380739qke.108.2021.01.04.15.46.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Jan 2021 15:46:52 -0800 (PST) Date: Mon, 4 Jan 2021 16:46:51 -0700 From: Nathan Chancellor To: Dennis Zhou Cc: Tejun Heo , Christoph Lameter , linux-mm@kvack.org, linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com, kbuild-all@lists.01.org, kernel test robot Subject: Re: [PATCH] percpu: fix clang modpost warning in pcpu_build_alloc_info() Message-ID: <20210104234651.GA3548546@ubuntu-m3-large-x86> References: <20201231212852.3175381-1-dennis@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201231212852.3175381-1-dennis@kernel.org> 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 Thu, Dec 31, 2020 at 09:28:52PM +0000, Dennis Zhou wrote: > This is an unusual situation so I thought it best to explain it in a > separate patch. > > "percpu: reduce the number of cpu distance comparisons" introduces a > dependency on cpumask helper functions in __init code. This code > references a struct cpumask annotated __initdata. When the function is > inlined (gcc), everything is fine, but clang decides not to inline these > function calls. This causes modpost to warn about an __initdata access > by a function not annotated with __init [1]. > > Ways I thought about fixing it: > 1. figure out why clang thinks this inlining is too costly. > 2. create a wrapper function annotated __init (this). > 3. annotate cpumask with __refdata. > > Ultimately it comes down to if it's worth saving the cpumask memory and > allowing it to be freed. IIUC, __refdata won't be freed, so option 3 is > just a little wasteful. 1 is out of my depth, leaving 2. I don't feel > great about this behavior being dependent on inlining semantics, but > cpumask helpers are small and probably should be inlined. > > modpost complaint: > WARNING: modpost: vmlinux.o(.text+0x735425): 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. > > clang output: > mm/percpu.c:2724:5: remark: cpumask_clear_cpu not inlined into pcpu_build_alloc_info because too costly to inline (cost=725, threshold=325) [-Rpass-missed=inline] > > [1] https://lore.kernel.org/linux-mm/202012220454.9F6Bkz9q-lkp@intel.com/ > > Reported-by: kernel test robot > Signed-off-by: Dennis Zhou > --- > This is on top of percpu#for-5.12. > > mm/percpu.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/mm/percpu.c b/mm/percpu.c > index 80f8f885a990..357977c4cb00 100644 > --- a/mm/percpu.c > +++ b/mm/percpu.c > @@ -2642,6 +2642,18 @@ early_param("percpu_alloc", percpu_alloc_setup); > > /* pcpu_build_alloc_info() is used by both embed and page first chunk */ > #if defined(BUILD_EMBED_FIRST_CHUNK) || defined(BUILD_PAGE_FIRST_CHUNK) > + > +/* > + * This wrapper is to avoid a warning where cpumask_clear_cpu() is not inlined > + * when compiling with clang causing modpost to warn about accessing __initdata > + * from a non __init function. By doing this, we allow the struct cpumask to be > + * freed instead of it taking space by annotating with __refdata. > + */ > +static void __init pcpu_cpumask_clear_cpu(int cpu, struct cpumask *mask) > +{ > + cpumask_clear_cpu(cpu, mask); > +} > + > /** > * pcpu_build_alloc_info - build alloc_info considering distances between CPUs > * @reserved_size: the size of reserved percpu area in bytes > @@ -2713,7 +2725,7 @@ static struct pcpu_alloc_info * __init pcpu_build_alloc_info( > cpu = cpumask_first(&mask); > group_map[cpu] = group; > group_cnt[group]++; > - cpumask_clear_cpu(cpu, &mask); > + pcpu_cpumask_clear_cpu(cpu, &mask); > > for_each_cpu(tcpu, &mask) { > if (!cpu_distance_fn || > @@ -2721,7 +2733,7 @@ static struct pcpu_alloc_info * __init pcpu_build_alloc_info( > cpu_distance_fn(tcpu, cpu) == LOCAL_DISTANCE)) { > group_map[tcpu] = group; > group_cnt[group]++; > - cpumask_clear_cpu(tcpu, &mask); > + pcpu_cpumask_clear_cpu(tcpu, &mask); > } > } > } > -- > 2.29.2.729.g45daf8777d-goog > 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. 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? Cheers, Nathan