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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 604C5C433EF for ; Tue, 22 Mar 2022 16:32:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CD3CE6B0072; Tue, 22 Mar 2022 12:32:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C82856B0074; Tue, 22 Mar 2022 12:32:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B232D6B0075; Tue, 22 Mar 2022 12:32:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0085.hostedemail.com [216.40.44.85]) by kanga.kvack.org (Postfix) with ESMTP id A48BD6B0072 for ; Tue, 22 Mar 2022 12:32:32 -0400 (EDT) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 58D338248D52 for ; Tue, 22 Mar 2022 16:32:32 +0000 (UTC) X-FDA: 79272565344.19.BDD5680 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by imf30.hostedemail.com (Postfix) with ESMTP id 99D8480022 for ; Tue, 22 Mar 2022 16:32:31 +0000 (UTC) Received: by mail-pl1-f177.google.com with SMTP id j13so6015809plj.8 for ; Tue, 22 Mar 2022 09:32:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=8VW7m+2y1Tmk6nQ2gEGQ8MmmBh8Dx2qsCve3MAkxzOA=; b=iP3chaaFb25Z76RNWTQRRmAxgT63t60OG13XsvAYeYEeIOU2ZSnJLLJs72Q2MYrkQe phrwr4nVFXhftWilaIaVrFuQLHWBDqhI6XUZnIMuEfkzIJLo7+AVGN5PLBcxxqka5Le7 d7brDHFCF289t4v1ip9FKpt9yNWwdVT8daQWGOJKJoLjwDWyUswCGfUtI7v4tem4sx/3 vuDLcYp2TL4ogSyeEC6bJGqqUCEu6VcLdvfbYYurkUBVmyVt3in9BH/ic6tqY5ca8ngU 9BCsvMrCN1W1QOCT4OzkrwgUoq0hIl1P9TEpIH9nQVn1ktfQbeCTnIARzhZ2jVkntUzk AFZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=8VW7m+2y1Tmk6nQ2gEGQ8MmmBh8Dx2qsCve3MAkxzOA=; b=GwBbKrMOI9DE0oE+VojA458kr7P5DwFnacEB+u2htBfk3cVOzaCphk3cugmMjgBkBH KgJL9+SsYeZ8+sKuGY6nml8N2vNRAu8H+qLEdL+Z1c1HV2fMBOGb2v55gjjUxEuz864b fMB0kluGnvv8ULxE/nEh64qJ7eHtBvEowzU+bE5jBeOXALh66qgg0Nl1bewCefogq+Dt J8EUvuvZ84txO3dAgBSbQ+fXV9mjp2zaXSjLxkgf9A89JK9AUZUNhKRBhkp5YQY24hyI L14zQgMBGOgQeUdcnuZCi8naiau5yNUJMF0ClZv5uu4wXWhwoP+t7G31uw8gQyTMpEMz t8bA== X-Gm-Message-State: AOAM530i2n04OpSb1+FJq3XwEU4WXjcS8pXruGNHSTIrdaf02iKR6qgQ 3RbIcvPROhHTEM6Uj2Wet0WcMMqqyH0lZoh+T+qsJw== X-Google-Smtp-Source: ABdhPJzwYAYZ1e3ypdIIPaNrrCcfQjyeMu+eJoTkNU8vKXqKPTzyE5ogynPUrk/F9IVMPrRJQHsNliZF1dcMQEc2/zE= X-Received: by 2002:a17:903:32c7:b0:154:4156:f384 with SMTP id i7-20020a17090332c700b001544156f384mr15091046plr.34.1647966750443; Tue, 22 Mar 2022 09:32:30 -0700 (PDT) MIME-Version: 1.0 References: <20220318114133.113627-1-kjain@linux.ibm.com> <20220318114133.113627-2-kjain@linux.ibm.com> In-Reply-To: From: Dan Williams Date: Tue, 22 Mar 2022 09:32:23 -0700 Message-ID: Subject: Re: [PATCH 2/2] powerpc/papr_scm: Fix build failure when CONFIG_PERF_EVENTS is not set To: kajoljain Cc: Michael Ellerman , linuxppc-dev , Linux NVDIMM , Linux Kernel Mailing List , Linux-Next Mailing List , Peter Zijlstra , "Weiny, Ira" , Vishal L Verma , Stephen Rothwell , Santosh Sivaraj , maddy@linux.ibm.com, rnsastry@linux.ibm.com, "Aneesh Kumar K.V" , atrajeev@linux.vnet.ibm.com, Vaibhav Jain , Thomas Gleixner , Linux MM Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: tjmkoupud4e93ktd1auiceh4msb7151w Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=intel-com.20210112.gappssmtp.com header.s=20210112 header.b=iP3chaaF; spf=none (imf30.hostedemail.com: domain of dan.j.williams@intel.com has no SPF policy when checking 209.85.214.177) smtp.mailfrom=dan.j.williams@intel.com; dmarc=fail reason="No valid SPF, DKIM not aligned (relaxed)" header.from=intel.com (policy=none) X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 99D8480022 X-HE-Tag: 1647966751-221428 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 Tue, Mar 22, 2022 at 7:30 AM kajoljain wrote: > > > > On 3/22/22 03:09, Dan Williams wrote: > > On Fri, Mar 18, 2022 at 4:42 AM Kajol Jain wrote: > >> > >> The following build failure occures when CONFIG_PERF_EVENTS is not set > >> as generic pmu functions are not visible in that scenario. > >> > >> arch/powerpc/platforms/pseries/papr_scm.c:372:35: error: =E2=80=98stru= ct perf_event=E2=80=99 has no member named =E2=80=98attr=E2=80=99 > >> p->nvdimm_events_map[event->attr.config], > >> ^~ > >> In file included from ./include/linux/list.h:5, > >> from ./include/linux/kobject.h:19, > >> from ./include/linux/of.h:17, > >> from arch/powerpc/platforms/pseries/papr_scm.c:5: > >> arch/powerpc/platforms/pseries/papr_scm.c: In function =E2=80=98papr_s= cm_pmu_event_init=E2=80=99: > >> arch/powerpc/platforms/pseries/papr_scm.c:389:49: error: =E2=80=98stru= ct perf_event=E2=80=99 has no member named =E2=80=98pmu=E2=80=99 > >> struct nvdimm_pmu *nd_pmu =3D to_nvdimm_pmu(event->pmu); > >> ^~ > >> ./include/linux/container_of.h:18:26: note: in definition of macro =E2= =80=98container_of=E2=80=99 > >> void *__mptr =3D (void *)(ptr); \ > >> ^~~ > >> arch/powerpc/platforms/pseries/papr_scm.c:389:30: note: in expansion o= f macro =E2=80=98to_nvdimm_pmu=E2=80=99 > >> struct nvdimm_pmu *nd_pmu =3D to_nvdimm_pmu(event->pmu); > >> ^~~~~~~~~~~~~ > >> In file included from ./include/linux/bits.h:22, > >> from ./include/linux/bitops.h:6, > >> from ./include/linux/of.h:15, > >> from arch/powerpc/platforms/pseries/papr_scm.c:5: > >> > >> Fix the build issue by adding check for CONFIG_PERF_EVENTS config opti= on > >> and disabling the papr_scm perf interface support incase this config > >> is not set > >> > >> Fixes: 4c08d4bbc089 ("powerpc/papr_scm: Add perf interface support") (= Commit id > >> based on linux-next tree) > >> Signed-off-by: Kajol Jain > >> --- > >> arch/powerpc/platforms/pseries/papr_scm.c | 15 +++++++++++++++ > > > > This is a bit messier than I would have liked mainly because it dumps > > a bunch of ifdefery into a C file contrary to coding style, "Wherever > > possible, don't use preprocessor conditionals (#if, #ifdef) in .c > > files". I would expect this all to move to an organization like: > > Hi Dan, > Thanks for reviewing the patches. Inorder to avoid the multiple > ifdefs checks, we can also add stub function for papr_scm_pmu_register. > With that change we will just have one ifdef check for > CONFIG_PERF_EVENTS config in both papr_scm.c and nd.h file. Hence we can > avoid adding new files specific for papr_scm perf interface. > > Below is the code snippet for that change, let me know if looks fine to > you. I tested it > with set/unset PAPR_SCM config value and set/unset PERF_EVENTS config > value combinations. > > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c > b/arch/powerpc/platforms/pseries/papr_scm.c > index 4dd513d7c029..38fabb44d3c3 100644 > --- a/arch/powerpc/platforms/pseries/papr_scm.c > +++ b/arch/powerpc/platforms/pseries/papr_scm.c > @@ -69,8 +69,6 @@ > #define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS) > #define PAPR_SCM_PERF_STATS_VERSION 0x1 > > -#define to_nvdimm_pmu(_pmu) container_of(_pmu, struct nvdimm_pmu, pmu= ) > - > /* Struct holding a single performance metric */ > struct papr_scm_perf_stat { > u8 stat_id[8]; > @@ -346,6 +344,9 @@ static ssize_t drc_pmem_query_stats(struct > papr_scm_priv *p, > return 0; > } > > +#ifdef CONFIG_PERF_EVENTS > +#define to_nvdimm_pmu(_pmu) container_of(_pmu, struct nvdimm_pmu, pmu= ) > + > static int papr_scm_pmu_get_value(struct perf_event *event, struct > device *dev, u64 *count) > { > struct papr_scm_perf_stat *stat; > @@ -558,6 +559,10 @@ static void papr_scm_pmu_register(struct > papr_scm_priv *p) > dev_info(&p->pdev->dev, "nvdimm pmu didn't register rc=3D%d\n", r= c); > } > > +#else > +static inline void papr_scm_pmu_register(struct papr_scm_priv *p) { } Since this isn't in a header file, it does not need to be marked "inline" the compiler will figure it out. > +#endif It might be time to create: arch/powerpc/platforms/pseries/papr_scm.h ...there is quite a bit of header material accrued in papr_scm.c and once the ifdefs start landing in it then it becomes a nominal coding style issue. That said, this is certainly more palatable than the previous version. So if you don't want to create papr_scm.h yet for this, at least make a note in the changelog that the first portion of arch/powerpc/platforms/pseries/papr_scm.c is effectively papr_scm.h content and may move there in the future, or something like that.