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

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

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

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

[Borislav Petkov]

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.

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

[Valdis Klētnieks]

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

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

[Borislav Petkov]

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.

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

[Borislav Petkov]

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.

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

[Valdis Klētnieks]

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

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

[Borislav Petkov]

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.

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

[Valdis Klētnieks]

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

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

[tip-bot for Valdis Kletnieks]

Commit-ID:  b6ff24f7b5101101ff897dfdde3f37924e676bc2
Gitweb:     https://git.kernel.org/tip/b6ff24f7b5101101ff897dfdde3f37924e676bc2
Author:     Valdis Kletnieks <valdis.kletnieks@vt.edu>
AuthorDate: Thu, 8 Aug 2019 16:32:27 +0200
Committer:  Borislav Petkov <bp@suse.de>
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:

  >> 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 <lkp@intel.com>
Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: linux-edac@vger.kernel.org
Cc: x86@kernel.org
Link: http://lkml.kernel.org/r/7053.1565218556@turing-police
---
 drivers/ras/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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