RE: linux-next: build warning in Linus' tree
> Hi all,
>
> On Thu, 18 Feb 2021 22:47:57 +0000 "Ernst, Justin"
> wrote:
> >
> > Hi,
> > We made a special effort to squash the unexpected indentation warnings in
> > c159376490ee
> (https://lore.kernel.org/lkml/20201130214304.369348-1-justin.er...@hpe.com/),
> so I was surprised to
> see this again.
> > Commit:
> >
> > c9624cb7db1c ("x86/platform/uv: Update sysfs documentation")
> >
> > is the culprit here. I suspect it was written and submitted before we made
> > the effort to fix the
> Unexpected indentation in c159376490ee, so it misplaced the first line of a
> codeblock, the original
> problem that was reported and fixed.
> >
> > The fix:
> >
> > diff --git a/Documentation/ABI/testing/sysfs-firmware-sgi_uv
> > b/Documentation/ABI/testing/sysfs-
> firmware-sgi_uv
> > index 637c668cbe45..12ed843e1d3e 100644
> > --- a/Documentation/ABI/testing/sysfs-firmware-sgi_uv
> > +++ b/Documentation/ABI/testing/sysfs-firmware-sgi_uv
> > @@ -39,8 +39,8 @@ Description:
> >
> > The uv_type entry contains the hub revision number.
> > This value can be used to identify the UV system version::
> > - "0.*" = Hubless UV ('*' is subtype)
> >
> > + "0.*" = Hubless UV ('*' is subtype)
> > "3.0" = UV2
> > "5.0" = UV3
> > "7.0" = UV4
> >
> > Thanks,
> > Justin
> >
> > > Building Linus' tree, today's linux-next build (htmldocs) produced
> > > this warning:
> > >
> > > Documentation/ABI/testing/sysfs-firmware-sgi_uv:2: WARNING: Unexpected
> > > indentation.
> > >
> > > Introduced by commit
> > >
> > > c159376490ee ("x86/platform/uv: Update ABI documentation of
> > > /sys/firmware/sgi_uv/")
> > >
> > > Or maybe an ealier one.
> > >
> > > This has been around for some time.
>
> I am still seeing this warning.
I submitted a patch here:
https://lore.kernel.org/lkml/20210219182852.385297-1-justin.er...@hpe.com/
Thanks,
Justin Ernst
>
> --
> Cheers,
> Stephen Rothwell
RE: linux-next: build warning in Linus' tree
Hi,
We made a special effort to squash the unexpected indentation warnings in
c159376490ee
(https://lore.kernel.org/lkml/20201130214304.369348-1-justin.er...@hpe.com/),
so I was surprised to see this again.
Commit:
c9624cb7db1c ("x86/platform/uv: Update sysfs documentation")
is the culprit here. I suspect it was written and submitted before we made the
effort to fix the Unexpected indentation in c159376490ee, so it misplaced the
first line of a codeblock, the original problem that was reported and fixed.
The fix:
diff --git a/Documentation/ABI/testing/sysfs-firmware-sgi_uv
b/Documentation/ABI/testing/sysfs-firmware-sgi_uv
index 637c668cbe45..12ed843e1d3e 100644
--- a/Documentation/ABI/testing/sysfs-firmware-sgi_uv
+++ b/Documentation/ABI/testing/sysfs-firmware-sgi_uv
@@ -39,8 +39,8 @@ Description:
The uv_type entry contains the hub revision number.
This value can be used to identify the UV system version::
- "0.*" = Hubless UV ('*' is subtype)
+ "0.*" = Hubless UV ('*' is subtype)
"3.0" = UV2
"5.0" = UV3
"7.0" = UV4
Thanks,
Justin
> Hi all,
>
> Building Linus' tree, today's linux-next build (htmldocs) produced
> this warning:
>
> Documentation/ABI/testing/sysfs-firmware-sgi_uv:2: WARNING: Unexpected
> indentation.
>
> Introduced by commit
>
> c159376490ee ("x86/platform/uv: Update ABI documentation of
> /sys/firmware/sgi_uv/")
>
> Or maybe an ealier one.
>
> This has been around for some time.
>
> --
> Cheers,
> Stephen Rothwell
RE: [PATCH 04/10] ABI: sysfs-firmware-sgi_uv
> From: Mauro Carvalho Chehab [mailto:mche...@kernel.org] On Behalf Of Mauro
> Carvalho Chehab
> Sent: Thursday, January 14, 2021 1:54 AM
> Subject: [PATCH 04/10] ABI: sysfs-firmware-sgi_uv
>
> Add a missing blank line required to identify a literal block,
> fixing this warning:
>
> .../Documentation/ABI/testing/sysfs-firmware-sgi_uv:2: WARNING:
> Unexpected indentation.
>
> Signed-off-by: Mauro Carvalho Chehab
Thanks for finding and fixing this. I was able to replicate the warning and
confirm the fix.
Reviewed-by: Justin Ernst
> ---
> Documentation/ABI/testing/sysfs-firmware-sgi_uv | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-firmware-sgi_uv
> b/Documentation/ABI/testing/sysfs-
> firmware-sgi_uv
> index 637c668cbe45..b0f79a1d14b3 100644
> --- a/Documentation/ABI/testing/sysfs-firmware-sgi_uv
> +++ b/Documentation/ABI/testing/sysfs-firmware-sgi_uv
> @@ -39,6 +39,7 @@ Description:
>
> The uv_type entry contains the hub revision number.
> This value can be used to identify the UV system version::
> +
> "0.*" = Hubless UV ('*' is subtype)
>
> "3.0" = UV2
> --
> 2.29.2
RE: linux-next: build warnings after merge of the tip tree
> On Mon, Nov 30, 2020 at 06:05:03PM +1100, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the tip tree, today's linux-next build (htmldocs) produced
> > these warnings:
> >
> > Documentation/ABI/testing/sysfs-firmware-sgi_uv:2: WARNING: Unexpected
> > indentation.
> > Documentation/ABI/testing/sysfs-firmware-sgi_uv:2: WARNING: Unexpected
> > indentation.
> > Documentation/ABI/testing/sysfs-firmware-sgi_uv:2: WARNING: Unexpected
> > indentation.
> >
> > Introduced by commit
> >
> > 7ac2f1017115 ("x86/platform/uv: Update ABI documentation of
> > /sys/firmware/sgi_uv/")
>
> Yah, I can reproduce but I have no clue what sphinx wants from me. Line
> 2 looks ok which could mean that the warning line it points to is bogus.
>
> Justin, this is all yours. :)
After scratching my head for a while, I found that the issue was missing empty
lines before three different code-block sections.
The line number is definitely bogus, but I wasn't able to discover why.
You can find my patch at: https://lkml.org/lkml/2020/11/30/1196
The patch depends on the v2 patch set Mike Travis
submitted, which hasn't made it to tip yet.
Thanks,
-Justin
>
> Thx.
>
> --
> Regards/Gruss,
> Boris.
>
> SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG
> Nürnberg
RE: [PATCH v2 0/5] Add uv_sysfs platform driver
Hans,
Thank you for your Ack of my patch set.
I've found a couple bugs that need fixing:
1. In my re-introduction of /sys/firmware/sgi_uv/coherence_id, I failed to
export the associated sn_coherency_id variable, causing the build to fail when
UV_SYSFS=m
2. A null pointer dereference in
drivers/platform/x86/uv_sysfs.c:uv_ports_exit() caused by calling kobject_put()
on an out of range index value.
I will be resubmitting the patch series shortly as v3.
I apologize for the hiccup.
Thanks,
Justin
> -Original Message-
> From: Hans de Goede [mailto:hdego...@redhat.com]
> Sent: Tuesday, November 24, 2020 5:30 AM
> To: Ernst, Justin ; Borislav Petkov ;
> Ingo Molnar
> ; Mark Gross ; Thomas Gleixner
> ; Wahl,
> Steve ; x...@kernel.org
> Cc: Andy Shevchenko ; Darren Hart ;
> Sivanich, Dimitri
> ; H . Peter Anvin ; Anderson, Russ
> ;
> linux-kernel@vger.kernel.org; platform-driver-...@vger.kernel.org; Cezary
> Rojewski
> ; Ilya Dryomov ; Jonathan
> Cameron
> ; Mauro Carvalho Chehab
> ; Vaibhav Jain
>
> Subject: Re: [PATCH v2 0/5] Add uv_sysfs platform driver
>
> Hi,
>
> Quick self intro for the x86/tip maintainers: I have take over
> drivers/platform/x86 maintainership from Andy.
>
> On 11/18/20 5:47 PM, Justin Ernst wrote:
> > Introduce a new platform driver to gather topology information from UV
> > systems
> > and expose that information via a sysfs interface at /sys/firmware/sgi_uv/.
> >
> > This is version 2 with these changes since version 1:
> >
> > * Re-introduced /sys/firmware/sgi_uv/coherence_id file in the new driver
> > after
> > removing it in Patch 1/5. This keeps the userspace API unbroken.
> >
> > Justin Ernst (5):
> > x86/platform/uv: Remove existing /sys/firmware/sgi_uv/ interface
> > x86/platform/uv: Add and export uv_bios_* functions
> > x86/platform/uv: Add new uv_sysfs platform driver
> > x86/platform/uv: Update ABI documentation of /sys/firmware/sgi_uv/
> > x86/platform/uv: Update MAINTAINERS for uv_sysfs driver>
> > .../ABI/testing/sysfs-firmware-sgi_uv | 141 ++-
> > MAINTAINERS | 6 +
> > arch/x86/include/asm/uv/bios.h| 49 +
> > arch/x86/include/asm/uv/uv_geo.h | 103 +++
> > arch/x86/platform/uv/Makefile | 2 +-
> > arch/x86/platform/uv/bios_uv.c| 54 ++
> > arch/x86/platform/uv/uv_sysfs.c | 63 --
> > drivers/platform/x86/Kconfig | 11 +
> > drivers/platform/x86/Makefile | 3 +
> > drivers/platform/x86/uv_sysfs.c | 862 ++
> > 10 files changed, 1216 insertions(+), 78 deletions(-)
> > create mode 100644 arch/x86/include/asm/uv/uv_geo.h
> > delete mode 100644 arch/x86/platform/uv/uv_sysfs.c
> > create mode 100644 drivers/platform/x86/uv_sysfs.c
>
> So this touches files under both arch/x86/ and drivers/platform/x86/ ,
> I believe this is best merged through the x86/tip tree and I don't
> expect conflicts for the drivers/platform/x86/{Kconfig,Makefile} changes.
>
> So here is my ack for merging this series through the x86/tip tree:
>
> Acked-by: Hans de Goede
>
> Regards,
>
> Hans
>
RE: [PATCH 0/5] Add uv_sysfs platform driver
> Hi, > > On 11/17/20 9:42 PM, Justin Ernst wrote: > > Introduce a new platform driver to gather topology information from UV > > systems > > and expose that information via a sysfs interface at /sys/firmware/sgi_uv/. > > > > Justin Ernst (5): > > x86/platform/uv: Remove existing /sys/firmware/sgi_uv/ interface > > x86/platform/uv: Add and export uv_bios_* functions > > x86/platform/uv: Add new uv_sysfs platform driver > > x86/platform/uv: Update ABI documentation of /sys/firmware/sgi_uv/ > > x86/platform/uv: Update MAINTAINERS for uv_sysfs driver > > So patch 1/1 drops the existing > > /sys/firmware/sgi_uv/coherence_id > /sys/firmware/sgi_uv/partition_id > > sysfs API, then according to patch 4/5 patch 3/5 reintroduces > the /sys/firmware/sgi_uv/partition_id API, but the > /sys/firmware/sgi_uv/coherence_id > file is gone for ever ? > > I'm not sure what userspace bits (may) depend on this but without more info > this looks like a clear violation of the do not break userspace APIs rule. > > So, based on this, I have to nack this series in its current state. > > Now if there is a strong believe there are 0 (not a few, but _zero_) users > out there who rely on the /sys/firmware/sgi_uv/coherence_id file then this > might be ok. But then there needs to be a technical analysis of why this is > ok in the commit message of the patch dropping this sysfs file. > > Also the commit message of patch 1/5 should mention that > /sys/firmware/sgi_uv/partition_id will be re-introduced later through > another driver. Hello Hans, I will resubmit these patches without the API breakage, reintroducing the coherence_id file in the new driver. Thank you for taking the time to look over my patch set. > > Regards, > > Hans
RE: [PATCH] EDAC: Don't add devices under /sys/bus/edac
Looks good on a 32 socket system. All 64 memory controllers show up and I'm able to see the same sysfs diff. Thanks -Justin > -Original Message- > From: Borislav Petkov [mailto:b...@alien8.de] > Sent: Tuesday, November 6, 2018 8:46 AM > To: Ernst, Justin ; Luck, Tony > Cc: Greg KH ; Anderson, Russ > ; Mauro Carvalho Chehab > ; linux-e...@vger.kernel.org; linux- > ker...@vger.kernel.org; Aristeu Rozanski Filho > Subject: Re: [PATCH] EDAC: Don't add devices under /sys/bus/edac > > On Tue, Oct 02, 2018 at 06:26:08PM +0200, Borislav Petkov wrote: > > On Tue, Oct 02, 2018 at 03:51:41PM +, Ernst, Justin wrote: > > > The combined patches work on a 20 socket system. > > > Thanks! > > > > Cool, thanks for testing. > > > > Nevertheless, I'll queue them for 4.21 so that we have a full cycle of > > testing before we really kill the bus thing. > > Ok, I've pushed the two patches ontop of EDAC's for-next branch, here: > > https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=edac-for- > 4.21-bus > > and I'd appreciate testing them one more time on the big boxes you guys > have. > > Diffing sysfs here shows this: > > --- edac.before 2018-11-06 13:37:37.925448609 +0100 > +++ edac.after2018-11-06 15:36:11.229497795 +0100 > @@ -37,7 +37,6 @@ > /sys/devices/system/edac/mc/mc0/dimm3/power/control > /sys/devices/system/edac/mc/mc0/dimm3/dimm_dev_type > /sys/devices/system/edac/mc/mc0/dimm3/size > -/sys/devices/system/edac/mc/mc0/dimm3/subsystem > /sys/devices/system/edac/mc/mc0/dimm3/dimm_ce_count > /sys/devices/system/edac/mc/mc0/dimm3/dimm_label > /sys/devices/system/edac/mc/mc0/dimm3/dimm_location > > which is the bus symlink: > > subsystem -> ../../../../../../bus/mc0 > > and I think that's ok as nothing should be using it. > > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply.
RE: [PATCH] EDAC: Don't add devices under /sys/bus/edac
The combined patches work on a 20 socket system.
Thanks!
-Justin
> -Original Message-
> From: Mauro Carvalho Chehab [mailto:mchehab+sams...@kernel.org]
> Sent: Monday, October 1, 2018 8:23 PM
> To: Luck, Tony
> Cc: Borislav Petkov ; Anderson, Russ
> ; Greg KH ; Ernst,
> Justin ; Anderson, Russ ;
> Mauro Carvalho Chehab ; linux-
> e...@vger.kernel.org; linux-kernel@vger.kernel.org; Aristeu Rozanski Filho
>
> Subject: Re: [PATCH] EDAC: Don't add devices under /sys/bus/edac
>
> Em Mon, 1 Oct 2018 15:43:13 -0700
> "Luck, Tony" escreveu:
>
> > Nobody(*) uses them. Dropping this will allow us to make the total
> > number of memory controllers configurable (as we won't have to
> > worry about duplicated device names under this directory).
> >
> > (*) https://marc.info/?l=linux-edac&m=153809709903987&w=2
> >
> > Signed-off-by: Tony Luck
> > ---
> >
> > Boris: Apply this, then your earlier patch to get rid of the
> > hard coded limit on the number of memory controllers:
> > https://marc.info/?l=linux-edac&m=153797567628947&w=2
> > the combination works on my 4 socket machine. Perhaps HPE
> > can test on their superdome.
> >
>
> For both this and the referred patch:
>
> Acked-by: Mauro Carvalho Chehab
>
> > drivers/edac/edac_mc_sysfs.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
> > index 20374b8248f0..4c1bee59c2e6 100644
> > --- a/drivers/edac/edac_mc_sysfs.c
> > +++ b/drivers/edac/edac_mc_sysfs.c
> > @@ -405,7 +405,6 @@ static int edac_create_csrow_object(struct
> mem_ctl_info *mci,
> > struct csrow_info *csrow, int index)
> > {
> > csrow->dev.type = &csrow_attr_type;
> > - csrow->dev.bus = mci->bus;
> > csrow->dev.groups = csrow_dev_groups;
> > device_initialize(&csrow->dev);
> > csrow->dev.parent = &mci->dev;
> > @@ -636,7 +635,6 @@ static int edac_create_dimm_object(struct
> mem_ctl_info *mci,
> > dimm->mci = mci;
> >
> > dimm->dev.type = &dimm_attr_type;
> > - dimm->dev.bus = mci->bus;
> > device_initialize(&dimm->dev);
> >
> > dimm->dev.parent = &mci->dev;
> > @@ -940,7 +938,6 @@ int edac_create_sysfs_mci_device(struct
> mem_ctl_info *mci,
> > device_initialize(&mci->dev);
> >
> > mci->dev.parent = mci_pdev;
> > - mci->dev.bus = mci->bus;
> > mci->dev.groups = groups;
> > dev_set_name(&mci->dev, "mc%d", mci->mc_idx);
> > dev_set_drvdata(&mci->dev, mci);
>
>
>
> Thanks,
> Mauro
