| Message ID | 20240813121237.2382534-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] fault-inject: improve build for CONFIG_FAULT_INJECTION=n | expand |
On 13-08-2024 17:42, Jani Nikula wrote: > The fault-inject.h users across the kernel need to add a lot of #ifdef > CONFIG_FAULT_INJECTION to cater for shortcomings in the header. Make > fault-inject.h self-contained for CONFIG_FAULT_INJECTION=n, and add > stubs for DECLARE_FAULT_ATTR(), setup_fault_attr(), should_fail_ex(), > and should_fail() to allow removal of conditional compilation. Would a "Fixes" tag be appropriate here? Fixes: 6ff1cb355e62 ("[PATCH] fault-injection capabilities infrastructure") > > Cc: Akinobu Mita <akinobu.mita@gmail.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > include/linux/fault-inject.h | 36 +++++++++++++++++++++++++++++------- > 1 file changed, 29 insertions(+), 7 deletions(-) > > diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h > index 354413950d34..8c829d28dcf3 100644 > --- a/include/linux/fault-inject.h > +++ b/include/linux/fault-inject.h > @@ -2,13 +2,17 @@ > #ifndef _LINUX_FAULT_INJECT_H > #define _LINUX_FAULT_INJECT_H > > +#include <linux/err.h> > +#include <linux/types.h> > + > +struct dentry; > +struct kmem_cache; > + > #ifdef CONFIG_FAULT_INJECTION > > -#include <linux/types.h> > -#include <linux/debugfs.h> > +#include <linux/atomic.h> > #include <linux/configfs.h> > #include <linux/ratelimit.h> > -#include <linux/atomic.h> > > /* > * For explanation of the elements of this struct, see > @@ -51,6 +55,28 @@ int setup_fault_attr(struct fault_attr *attr, char *str); > bool should_fail_ex(struct fault_attr *attr, ssize_t size, int flags); > bool should_fail(struct fault_attr *attr, ssize_t size); > > +#else /* CONFIG_FAULT_INJECTION */ > + > +struct fault_attr { > +}; > + > +#define DECLARE_FAULT_ATTR(name) struct fault_attr name = {} > + > +static inline int setup_fault_attr(struct fault_attr *attr, char *str) > +{ > + return 0; /* Note: 0 means error for __setup() handlers! */ > +} > +static inline bool should_fail_ex(struct fault_attr *attr, ssize_t size, int flags) > +{ > + return false; > +} > +static inline bool should_fail(struct fault_attr *attr, ssize_t size) > +{ > + return false; > +} > + > +#endif /* CONFIG_FAULT_INJECTION */ > + > #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS > > struct dentry *fault_create_debugfs_attr(const char *name, > @@ -87,10 +113,6 @@ static inline void fault_config_init(struct fault_config *config, > > #endif /* CONFIG_FAULT_INJECTION_CONFIGFS */ > > -#endif /* CONFIG_FAULT_INJECTION */ > - > -struct kmem_cache; > - > #ifdef CONFIG_FAIL_PAGE_ALLOC > bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order); > #else
On Tue, 13 Aug 2024 15:12:35 +0300 Jani Nikula <jani.nikula@intel.com> wrote: > The fault-inject.h users across the kernel need to add a lot of #ifdef > CONFIG_FAULT_INJECTION to cater for shortcomings in the header. Make > fault-inject.h self-contained for CONFIG_FAULT_INJECTION=n, and add > stubs for DECLARE_FAULT_ATTR(), setup_fault_attr(), should_fail_ex(), > and should_fail() to allow removal of conditional compilation. > > --- a/include/linux/fault-inject.h > +++ b/include/linux/fault-inject.h > > -#include <linux/types.h> > -#include <linux/debugfs.h> Removing a nested include exposes all those sites which were erroneously depending upon that nested include. Here's what I have found so far, there will be more. --- a/mm/failslab.c~fault-inject-improve-build-for-config_fault_injection=n-fix +++ a/mm/failslab.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include <linux/fault-inject.h> #include <linux/error-injection.h> +#include <linux/debugfs.h> #include <linux/slab.h> #include <linux/mm.h> #include "slab.h" --- a/lib/fault-inject.c~fault-inject-improve-build-for-config_fault_injection=n-fix +++ a/lib/fault-inject.c @@ -2,6 +2,7 @@ #include <linux/kernel.h> #include <linux/init.h> #include <linux/random.h> +#include <linux/debugfs.h> #include <linux/sched.h> #include <linux/stat.h> #include <linux/types.h> --- a/kernel/futex/core.c~fault-inject-improve-build-for-config_fault_injection=n-fix +++ a/kernel/futex/core.c @@ -34,6 +34,7 @@ #include <linux/compat.h> #include <linux/jhash.h> #include <linux/pagemap.h> +#include <linux/debugfs.h> #include <linux/plist.h> #include <linux/memblock.h> #include <linux/fault-inject.h>
On Tue, 13 Aug 2024, Andrew Morton <akpm@linux-foundation.org> wrote: > On Tue, 13 Aug 2024 15:12:35 +0300 Jani Nikula <jani.nikula@intel.com> wrote: > >> The fault-inject.h users across the kernel need to add a lot of #ifdef >> CONFIG_FAULT_INJECTION to cater for shortcomings in the header. Make >> fault-inject.h self-contained for CONFIG_FAULT_INJECTION=n, and add >> stubs for DECLARE_FAULT_ATTR(), setup_fault_attr(), should_fail_ex(), >> and should_fail() to allow removal of conditional compilation. >> >> --- a/include/linux/fault-inject.h >> +++ b/include/linux/fault-inject.h >> >> -#include <linux/types.h> >> -#include <linux/debugfs.h> > > Removing a nested include exposes all those sites which were > erroneously depending upon that nested include. Here's what I have > found so far, there will be more. Right. I didn't hit them with the configs I tried... though I wonder why not, especially lib/fault-inject.c puzzles me. How do you want to proceed? Arguably uncovering and fixing those places is good, but that's kind of an unintended consequence here. BR, Jani. > > --- a/mm/failslab.c~fault-inject-improve-build-for-config_fault_injection=n-fix > +++ a/mm/failslab.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <linux/fault-inject.h> > #include <linux/error-injection.h> > +#include <linux/debugfs.h> > #include <linux/slab.h> > #include <linux/mm.h> > #include "slab.h" > --- a/lib/fault-inject.c~fault-inject-improve-build-for-config_fault_injection=n-fix > +++ a/lib/fault-inject.c > @@ -2,6 +2,7 @@ > #include <linux/kernel.h> > #include <linux/init.h> > #include <linux/random.h> > +#include <linux/debugfs.h> > #include <linux/sched.h> > #include <linux/stat.h> > #include <linux/types.h> > --- a/kernel/futex/core.c~fault-inject-improve-build-for-config_fault_injection=n-fix > +++ a/kernel/futex/core.c > @@ -34,6 +34,7 @@ > #include <linux/compat.h> > #include <linux/jhash.h> > #include <linux/pagemap.h> > +#include <linux/debugfs.h> > #include <linux/plist.h> > #include <linux/memblock.h> > #include <linux/fault-inject.h> > _ >
On Wed, 14 Aug 2024 09:57:31 +0300 Jani Nikula <jani.nikula@intel.com> wrote: > > Removing a nested include exposes all those sites which were > > erroneously depending upon that nested include. Here's what I have > > found so far, there will be more. > > Right. I didn't hit them with the configs I tried... though I wonder why > not, especially lib/fault-inject.c puzzles me. > > How do you want to proceed? Arguably uncovering and fixing those places > is good, but that's kind of an unintended consequence here. Is OK, it's a good change. I fixed everything which my usual build testing covers. Other things will be reported and I'll fix those also. If you have time to eyeball the 36 files which include fault-inject.h then that would help things along.
diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h index 354413950d34..8c829d28dcf3 100644 --- a/include/linux/fault-inject.h +++ b/include/linux/fault-inject.h @@ -2,13 +2,17 @@ #ifndef _LINUX_FAULT_INJECT_H #define _LINUX_FAULT_INJECT_H +#include <linux/err.h> +#include <linux/types.h> + +struct dentry; +struct kmem_cache; + #ifdef CONFIG_FAULT_INJECTION -#include <linux/types.h> -#include <linux/debugfs.h> +#include <linux/atomic.h> #include <linux/configfs.h> #include <linux/ratelimit.h> -#include <linux/atomic.h> /* * For explanation of the elements of this struct, see @@ -51,6 +55,28 @@ int setup_fault_attr(struct fault_attr *attr, char *str); bool should_fail_ex(struct fault_attr *attr, ssize_t size, int flags); bool should_fail(struct fault_attr *attr, ssize_t size); +#else /* CONFIG_FAULT_INJECTION */ + +struct fault_attr { +}; + +#define DECLARE_FAULT_ATTR(name) struct fault_attr name = {} + +static inline int setup_fault_attr(struct fault_attr *attr, char *str) +{ + return 0; /* Note: 0 means error for __setup() handlers! */ +} +static inline bool should_fail_ex(struct fault_attr *attr, ssize_t size, int flags) +{ + return false; +} +static inline bool should_fail(struct fault_attr *attr, ssize_t size) +{ + return false; +} + +#endif /* CONFIG_FAULT_INJECTION */ + #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS struct dentry *fault_create_debugfs_attr(const char *name, @@ -87,10 +113,6 @@ static inline void fault_config_init(struct fault_config *config, #endif /* CONFIG_FAULT_INJECTION_CONFIGFS */ -#endif /* CONFIG_FAULT_INJECTION */ - -struct kmem_cache; - #ifdef CONFIG_FAIL_PAGE_ALLOC bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order); #else
The fault-inject.h users across the kernel need to add a lot of #ifdef CONFIG_FAULT_INJECTION to cater for shortcomings in the header. Make fault-inject.h self-contained for CONFIG_FAULT_INJECTION=n, and add stubs for DECLARE_FAULT_ATTR(), setup_fault_attr(), should_fail_ex(), and should_fail() to allow removal of conditional compilation. Cc: Akinobu Mita <akinobu.mita@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- include/linux/fault-inject.h | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-)