drivers/ras: Don't build debugfs.o if no debugfs in config

PATCH In review Valdis Klētnieks 3 participants 9 messages Aug 7, 2019 – Aug 8, 2019

Subsystems (2)

EDAC-core RAS infrastructure

lore

Share & tools

lore

Valdis Klētnieks

author

2019-08-07 22:56 UTC #0 lore raw

PATCH v2 1/2 drivers/ras: Don't build debugfs.o if no debugfs in config

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 <[email protected]>

drivers/ras/Makefile +4 -1

index ef6777e14d3d..07a5c391cc23 100644
@@ -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

Borislav Petkov

maintainer

2019-08-08 09:30 UTC #1 lore raw

Quoted 10 lines · Valdis Klētnieks

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 <[email protected]>
drivers/ras/Makefile +0 -0
index ef6777e14d3d..07a5c391cc23 100644

drivers/ras/Makefile line 6

Quoted 7 lines

drivers/ras/Makefile +4 -1

@@ -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.

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Valdis Kl=?utf-8?Q?=c4=93?=tnieks

author

2019-08-08 13:01 UTC #2 lore raw

Quoted 5 lines · Borislav Petkov

Quoted 5 lines
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 <[email protected]>

Quoted 15 lines

drivers/ras/Makefile line 6
Quoted 12 lines
drivers/ras/Makefile +4 -1
index ef6777e14d3d..07a5c391cc23 100644
@@ -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....

Borislav Petkov

maintainer

2019-08-08 14:20 UTC #3 lore raw

Quoted 1 line · Valdis Klētnieks

It's needed if somebody applies the patch 2/2 -

It is needed for what?

Quoted 2 lines

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...

Quoted 3 lines

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
						^^^^^^^^

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Borislav Petkov

maintainer

2019-08-08 14:34 UTC #4 lore raw

Quoted 4 lines · Borislav Petkov

Quoted 1 line · Valdis Klētnieks
It's needed if somebody applies the patch 2/2 -
It is needed for what?

Nevermind, saw the 0day build bot mail.

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Valdis Kl=?utf-8?Q?=c4=93?=tnieks

author

2019-08-08 15:08 UTC #5 lore raw

In reply to Borislav Petkov

Quoted 3 lines · Borislav Petkov

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.

Borislav Petkov

maintainer

2019-08-08 15:13 UTC #6 lore raw

Quoted 9 lines · Valdis Klētnieks

Quoted 3 lines · Borislav Petkov
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:

---

drivers/ras/Makefile +2 -1

index ef6777e14d3d..6f0404f50107 100644
@@ -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.

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Valdis Kl=?utf-8?Q?=c4=93?=tnieks

author

2019-08-08 15:19 UTC #7 lore raw

drivers/ras/Makefile line 3

Quoted 6 lines · Borislav Petkov

I'm build-testing a slightly different version of yours and I'll commit it if it passes the build smoke tests:
drivers/ras/Makefile +2 -1
-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 :)

tip-bot for Valdis Kletnieks

merge bot

2019-08-08 15:50 UTC #8 lore raw

[tip:ras/core] RAS: Build debugfs.o only when enabled in Kconfig

In reply to Valdis Klētnieks

RAS: Build debugfs.o only when enabled in Kconfig

Commit-ID:  b6ff24f7b5101101ff897dfdde3f37924e676bc2
Gitweb:     https://git.kernel.org/tip/b6ff24f7b5101101ff897dfdde3f37924e676bc2
Author:     Valdis Kletnieks <[email protected]>
AuthorDate: Thu, 8 Aug 2019 16:32:27 +0200
Committer:  Borislav Petkov <[email protected]>
CommitDate: Thu, 8 Aug 2019 17:44:02 +0200

RAS: Build debugfs.o only when enabled in Kconfig

In addition, the 0day bot reported this build error:

Quoted 1 line

Quoted 1 line
drivers/ras/debugfs.c:10:5: error: redefinition of 'ras_userspace_consumers'

      int ras_userspace_consumers(void)
          ^~~~~~~~~~~~~~~~~~~~~~~
     In file included from drivers/ras/debugfs.c:3:0:
     include/linux/ras.h:14:19: note: previous definition of 'ras_userspace_consumers' was here
      static inline int ras_userspace_consumers(void) { return 0; }
                      ^~~~~~~~~~~~~~~~~~~~~~~

for a riscv-specific .config where CONFIG_DEBUG_FS is not set. Fix all that by making debugfs.o depend on that define.

[ bp: Rewrite commit message. ]

Reported-by kbuild test robot <[email protected]>

Signed-off-by Valdis Kletnieks <[email protected]>

Signed-off-by Borislav Petkov <[email protected]>

Cc Tony Luck <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/7053.1565218556@turing-police open here

---
 drivers/ras/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/ras/Makefile +2 -1

index ef6777e14d3d..6f0404f50107 100644
@@ -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