Linux-EDAC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config
@ 2019-08-07 22:55 Valdis Klētnieks
  2019-08-08  9:31 ` Borislav Petkov
  0 siblings, 1 reply; 8+ messages in thread
From: Valdis Klētnieks @ 2019-08-07 22:55 UTC (permalink / raw)
  To: Tony Luck, Borislav Petkov; +Cc: linux-edac, linux-kernel

There's no reason to build the debugfs.o if the kernel config doesn't
even include CONFIG_DEBUG_FS

Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>

diff --git a/drivers/ras/Makefile b/drivers/ras/Makefile
index ef6777e14d3d..07a5c391cc23 100644
--- a/drivers/ras/Makefile
+++ b/drivers/ras/Makefile
@@ -1,3 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_RAS)	+= ras.o debugfs.o
+obj-$(CONFIG_RAS)	+= ras.o
+ifeq ($(CONFIG_DEBUG_FS),y)
+obj-$(CONFIG_RAS)	+= debugfs.o
+endif
 obj-$(CONFIG_RAS_CEC)	+= cec.o


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config
  2019-08-07 22:55 [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config Valdis Klētnieks
@ 2019-08-08  9:31 ` Borislav Petkov
  2019-08-08 13:01   ` Valdis Klētnieks
  0 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2019-08-08  9:31 UTC (permalink / raw)
  To: Valdis Klētnieks; +Cc: Tony Luck, linux-edac, linux-kernel

On Wed, Aug 07, 2019 at 06:55:56PM -0400, Valdis Klētnieks wrote:
> There's no reason to build the debugfs.o if the kernel config doesn't
> even include CONFIG_DEBUG_FS
> 
> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
> 
> diff --git a/drivers/ras/Makefile b/drivers/ras/Makefile
> index ef6777e14d3d..07a5c391cc23 100644
> --- a/drivers/ras/Makefile
> +++ b/drivers/ras/Makefile
> @@ -1,3 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> -obj-$(CONFIG_RAS)	+= ras.o debugfs.o
> +obj-$(CONFIG_RAS)	+= ras.o
> +ifeq ($(CONFIG_DEBUG_FS),y)
> +obj-$(CONFIG_RAS)	+= debugfs.o
> +endif
>  obj-$(CONFIG_RAS_CEC)	+= cec.o

If this is not causing real trouble then I'd prefer to keep it this way
because ras.c is pretty useless without the debugfs functionality.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config
  2019-08-08  9:31 ` Borislav Petkov
@ 2019-08-08 13:01   ` Valdis Klētnieks
  2019-08-08 14:20     ` Borislav Petkov
  0 siblings, 1 reply; 8+ messages in thread
From: Valdis Klētnieks @ 2019-08-08 13:01 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: Tony Luck, linux-edac, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

On Thu, 08 Aug 2019 11:31:01 +0200, Borislav Petkov said:
> > There's no reason to build the debugfs.o if the kernel config doesn't
> > even include CONFIG_DEBUG_FS
> >
> > Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
> >
> > diff --git a/drivers/ras/Makefile b/drivers/ras/Makefile
> > index ef6777e14d3d..07a5c391cc23 100644
> > --- a/drivers/ras/Makefile
> > +++ b/drivers/ras/Makefile
> > @@ -1,3 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> > -obj-$(CONFIG_RAS)	+= ras.o debugfs.o
> > +obj-$(CONFIG_RAS)	+= ras.o
> > +ifeq ($(CONFIG_DEBUG_FS),y)
> > +obj-$(CONFIG_RAS)	+= debugfs.o
> > +endif
> >  obj-$(CONFIG_RAS_CEC)	+= cec.o
>
> If this is not causing real trouble then I'd prefer to keep it this way
> because ras.c is pretty useless without the debugfs functionality.

It's needed if somebody applies the patch 2/2 - and I just got a note from
the kbuild test robot saying that happened....

And if it's that useless, maybe *more* needs to be done to ensure that
debugfs is enabled if ras is being built - possibly a Kconfig 'select' or something....


[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config
  2019-08-08 13:01   ` Valdis Klētnieks
@ 2019-08-08 14:20     ` Borislav Petkov
  2019-08-08 14:35       ` Borislav Petkov
  2019-08-08 15:08       ` Valdis Klētnieks
  0 siblings, 2 replies; 8+ messages in thread
From: Borislav Petkov @ 2019-08-08 14:20 UTC (permalink / raw)
  To: Valdis Klētnieks; +Cc: Tony Luck, linux-edac, linux-kernel

On Thu, Aug 08, 2019 at 09:01:39AM -0400, Valdis Klētnieks wrote:
> It's needed if somebody applies the patch 2/2 -

It is needed for what?

> and I just got a note from the kbuild test robot saying that
> happened....

Yes, I don't see any issues with 2/2 only applied. I could be missing
some aspect though...

> And if it's that useless, maybe *more* needs to be done to ensure that
> debugfs is enabled if ras is being built - possibly a Kconfig 'select'
> or something....

Or something:

config RAS_CEC
        depends on X86_MCE && MEMORY_FAILURE && DEBUG_FS
						^^^^^^^^
-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config
  2019-08-08 14:20     ` Borislav Petkov
@ 2019-08-08 14:35       ` Borislav Petkov
  2019-08-08 15:08       ` Valdis Klētnieks
  1 sibling, 0 replies; 8+ messages in thread
From: Borislav Petkov @ 2019-08-08 14:35 UTC (permalink / raw)
  To: Valdis Klētnieks; +Cc: Tony Luck, linux-edac, linux-kernel

On Thu, Aug 08, 2019 at 04:20:55PM +0200, Borislav Petkov wrote:
> On Thu, Aug 08, 2019 at 09:01:39AM -0400, Valdis Klētnieks wrote:
> > It's needed if somebody applies the patch 2/2 -
> 
> It is needed for what?

Nevermind, saw the 0day build bot mail.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config
  2019-08-08 14:20     ` Borislav Petkov
  2019-08-08 14:35       ` Borislav Petkov
@ 2019-08-08 15:08       ` Valdis Klētnieks
  2019-08-08 15:14         ` Borislav Petkov
  1 sibling, 1 reply; 8+ messages in thread
From: Valdis Klētnieks @ 2019-08-08 15:08 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: Tony Luck, linux-edac, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

On Thu, 08 Aug 2019 16:20:55 +0200, Borislav Petkov said:
> config RAS_CEC
>         depends on X86_MCE && MEMORY_FAILURE && DEBUG_FS
> 						^^^^^^^^

I'm willing to respin that patch that way instead - if cec.c is basically
pointless without debugfs, that's probably a good solution. My first read
of the code was that the debugfs support was "additional optional" code,
not "this is pointless without it" code.

[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config
  2019-08-08 15:08       ` Valdis Klētnieks
@ 2019-08-08 15:14         ` Borislav Petkov
  2019-08-08 15:18           ` Valdis Klētnieks
  0 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2019-08-08 15:14 UTC (permalink / raw)
  To: Valdis Klētnieks; +Cc: Tony Luck, linux-edac, linux-kernel

On Thu, Aug 08, 2019 at 11:08:49AM -0400, Valdis Klētnieks wrote:
> On Thu, 08 Aug 2019 16:20:55 +0200, Borislav Petkov said:
> > config RAS_CEC
> >         depends on X86_MCE && MEMORY_FAILURE && DEBUG_FS
> > 						^^^^^^^^
> 
> I'm willing to respin that patch that way instead - if cec.c is basically
> pointless without debugfs, that's probably a good solution. My first read
> of the code was that the debugfs support was "additional optional" code,
> not "this is pointless without it" code.

That's already there so no need.

I'm build-testing a slightly different version of yours and I'll commit
it if it passes the build smoke tests:

---
diff --git a/drivers/ras/Makefile b/drivers/ras/Makefile
index ef6777e14d3d..6f0404f50107 100644
--- a/drivers/ras/Makefile
+++ b/drivers/ras/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_RAS)	+= ras.o debugfs.o
+obj-$(CONFIG_RAS)	+= ras.o
+obj-$(CONFIG_DEBUG_FS)	+= debugfs.o
 obj-$(CONFIG_RAS_CEC)	+= cec.o
---

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config
  2019-08-08 15:14         ` Borislav Petkov
@ 2019-08-08 15:18           ` Valdis Klētnieks
  0 siblings, 0 replies; 8+ messages in thread
From: Valdis Klētnieks @ 2019-08-08 15:18 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: Tony Luck, linux-edac, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

On Thu, 08 Aug 2019 17:14:15 +0200, Borislav Petkov said:

> I'm build-testing a slightly different version of yours and I'll commit
> it if it passes the build smoke tests:

> -obj-$(CONFIG_RAS)	+= ras.o debugfs.o
> +obj-$(CONFIG_RAS)	+= ras.o
> +obj-$(CONFIG_DEBUG_FS)	+= debugfs.o

OK. We'll do it that way then :)

[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07 22:55 [PATCH v2 1/2] drivers/ras: Don't build debugfs.o if no debugfs in config Valdis Klētnieks
2019-08-08  9:31 ` Borislav Petkov
2019-08-08 13:01   ` Valdis Klētnieks
2019-08-08 14:20     ` Borislav Petkov
2019-08-08 14:35       ` Borislav Petkov
2019-08-08 15:08       ` Valdis Klētnieks
2019-08-08 15:14         ` Borislav Petkov
2019-08-08 15:18           ` Valdis Klētnieks

Linux-EDAC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-edac/0 linux-edac/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-edac linux-edac/ https://lore.kernel.org/linux-edac \
		linux-edac@vger.kernel.org linux-edac@archiver.kernel.org
	public-inbox-index linux-edac


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-edac


AGPL code for this site: git clone https://public-inbox.org/ public-inbox