[PATCH 1/3] fault-inject: improve build for CONFIG_FAULT_INJECTION=n

[PATCH 1/3] fault-inject: improve build for CONFIG_FAULT_INJECTION=n

[Jani Nikula]

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(-)

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

[PATCH 2/3] drm/msm: clean up fault injection usage

[Jani Nikula]

With the proper stubs in place in linux/fault-inject.h, we can remove a
bunch of conditional compilation for CONFIG_FAULT_INJECTION=n.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/msm/msm_debugfs.c | 2 --
 drivers/gpu/drm/msm/msm_drv.c     | 2 --
 drivers/gpu/drm/msm/msm_drv.h     | 4 ----
 3 files changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index 4494f6d1c7cb..7ab607252d18 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -357,12 +357,10 @@ void msm_debugfs_init(struct drm_minor *minor)
 	if (priv->kms && priv->kms->funcs->debugfs_init)
 		priv->kms->funcs->debugfs_init(priv->kms, minor);
 
-#ifdef CONFIG_FAULT_INJECTION
 	fault_create_debugfs_attr("fail_gem_alloc", minor->debugfs_root,
 				  &fail_gem_alloc);
 	fault_create_debugfs_attr("fail_gem_iova", minor->debugfs_root,
 				  &fail_gem_iova);
-#endif
 }
 #endif
 
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9c33f4e3f822..6938410f4fc7 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -58,10 +58,8 @@ static bool modeset = true;
 MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (1=on (default), 0=disable)");
 module_param(modeset, bool, 0600);
 
-#ifdef CONFIG_FAULT_INJECTION
 DECLARE_FAULT_ATTR(fail_gem_alloc);
 DECLARE_FAULT_ATTR(fail_gem_iova);
-#endif
 
 static int msm_drm_uninit(struct device *dev)
 {
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index be016d7b4ef1..9b953860131b 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -33,12 +33,8 @@
 #include <drm/msm_drm.h>
 #include <drm/drm_gem.h>
 
-#ifdef CONFIG_FAULT_INJECTION
 extern struct fault_attr fail_gem_alloc;
 extern struct fault_attr fail_gem_iova;
-#else
-#  define should_fail(attr, size) 0
-#endif
 
 struct msm_kms;
 struct msm_gpu;
-- 
2.39.2

Re: [PATCH 2/3] drm/msm: clean up fault injection usage

[Thomas Hellström]

On Tue, 2024-08-13 at 15:12 +0300, Jani Nikula wrote:
> With the proper stubs in place in linux/fault-inject.h, we can remove
> a
> bunch of conditional compilation for CONFIG_FAULT_INJECTION=n.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

LGTM.
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

> ---
>  drivers/gpu/drm/msm/msm_debugfs.c | 2 --
>  drivers/gpu/drm/msm/msm_drv.c     | 2 --
>  drivers/gpu/drm/msm/msm_drv.h     | 4 ----
>  3 files changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_debugfs.c
> b/drivers/gpu/drm/msm/msm_debugfs.c
> index 4494f6d1c7cb..7ab607252d18 100644
> --- a/drivers/gpu/drm/msm/msm_debugfs.c
> +++ b/drivers/gpu/drm/msm/msm_debugfs.c
> @@ -357,12 +357,10 @@ void msm_debugfs_init(struct drm_minor *minor)
>  	if (priv->kms && priv->kms->funcs->debugfs_init)
>  		priv->kms->funcs->debugfs_init(priv->kms, minor);
>  
> -#ifdef CONFIG_FAULT_INJECTION
>  	fault_create_debugfs_attr("fail_gem_alloc", minor-
> >debugfs_root,
>  				  &fail_gem_alloc);
>  	fault_create_debugfs_attr("fail_gem_iova", minor-
> >debugfs_root,
>  				  &fail_gem_iova);
> -#endif
>  }
>  #endif
>  
> diff --git a/drivers/gpu/drm/msm/msm_drv.c
> b/drivers/gpu/drm/msm/msm_drv.c
> index 9c33f4e3f822..6938410f4fc7 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -58,10 +58,8 @@ static bool modeset = true;
>  MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (1=on
> (default), 0=disable)");
>  module_param(modeset, bool, 0600);
>  
> -#ifdef CONFIG_FAULT_INJECTION
>  DECLARE_FAULT_ATTR(fail_gem_alloc);
>  DECLARE_FAULT_ATTR(fail_gem_iova);
> -#endif
>  
>  static int msm_drm_uninit(struct device *dev)
>  {
> diff --git a/drivers/gpu/drm/msm/msm_drv.h
> b/drivers/gpu/drm/msm/msm_drv.h
> index be016d7b4ef1..9b953860131b 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -33,12 +33,8 @@
>  #include <drm/msm_drm.h>
>  #include <drm/drm_gem.h>
>  
> -#ifdef CONFIG_FAULT_INJECTION
>  extern struct fault_attr fail_gem_alloc;
>  extern struct fault_attr fail_gem_iova;
> -#else
> -#  define should_fail(attr, size) 0
> -#endif
>  
>  struct msm_kms;
>  struct msm_gpu;

Re: [PATCH 2/3] drm/msm: clean up fault injection usage

[Ghimiray, Himal Prasad]

On 13-08-2024 17:42, Jani Nikula wrote:
> With the proper stubs in place in linux/fault-inject.h, we can remove a
> bunch of conditional compilation for CONFIG_FAULT_INJECTION=n.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/msm/msm_debugfs.c | 2 --
>   drivers/gpu/drm/msm/msm_drv.c     | 2 --
>   drivers/gpu/drm/msm/msm_drv.h     | 4 ----
>   3 files changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
> index 4494f6d1c7cb..7ab607252d18 100644
> --- a/drivers/gpu/drm/msm/msm_debugfs.c
> +++ b/drivers/gpu/drm/msm/msm_debugfs.c
> @@ -357,12 +357,10 @@ void msm_debugfs_init(struct drm_minor *minor)
>   	if (priv->kms && priv->kms->funcs->debugfs_init)
>   		priv->kms->funcs->debugfs_init(priv->kms, minor);
>   
> -#ifdef CONFIG_FAULT_INJECTION
>   	fault_create_debugfs_attr("fail_gem_alloc", minor->debugfs_root,
>   				  &fail_gem_alloc);
>   	fault_create_debugfs_attr("fail_gem_iova", minor->debugfs_root,
>   				  &fail_gem_iova);
> -#endif
>   }
>   #endif
>   
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 9c33f4e3f822..6938410f4fc7 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -58,10 +58,8 @@ static bool modeset = true;
>   MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (1=on (default), 0=disable)");
>   module_param(modeset, bool, 0600);
>   
> -#ifdef CONFIG_FAULT_INJECTION
>   DECLARE_FAULT_ATTR(fail_gem_alloc);
>   DECLARE_FAULT_ATTR(fail_gem_iova);
> -#endif
>   
>   static int msm_drm_uninit(struct device *dev)
>   {
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index be016d7b4ef1..9b953860131b 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -33,12 +33,8 @@
>   #include <drm/msm_drm.h>
>   #include <drm/drm_gem.h>
>   
> -#ifdef CONFIG_FAULT_INJECTION
>   extern struct fault_attr fail_gem_alloc;
>   extern struct fault_attr fail_gem_iova;
> -#else
> -#  define should_fail(attr, size) 0
> -#endif

LGTM
Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>

>   
>   struct msm_kms;
>   struct msm_gpu;

Re: [PATCH 2/3] drm/msm: clean up fault injection usage

[Abhinav Kumar]

On 8/13/2024 5:12 AM, Jani Nikula wrote:
> With the proper stubs in place in linux/fault-inject.h, we can remove a
> bunch of conditional compilation for CONFIG_FAULT_INJECTION=n.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/msm/msm_debugfs.c | 2 --
>   drivers/gpu/drm/msm/msm_drv.c     | 2 --
>   drivers/gpu/drm/msm/msm_drv.h     | 4 ----
>   3 files changed, 8 deletions(-)
> 

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

[PATCH 3/3] drm/xe: clean up fault injection usage

[Jani Nikula]

With the proper stubs in place in linux/fault-inject.h, we can remove a
bunch of conditional compilation for CONFIG_FAULT_INJECTION=n.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/xe/xe_debugfs.c |  7 +------
 drivers/gpu/drm/xe/xe_gt.h      | 10 ++--------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
index 1011e5d281fa..b381bfb634f7 100644
--- a/drivers/gpu/drm/xe/xe_debugfs.c
+++ b/drivers/gpu/drm/xe/xe_debugfs.c
@@ -6,6 +6,7 @@
 #include "xe_debugfs.h"
 
 #include <linux/debugfs.h>
+#include <linux/fault-inject.h>
 #include <linux/string_helpers.h>
 
 #include <drm/drm_debugfs.h>
@@ -26,10 +27,7 @@
 #include "xe_vm.h"
 #endif
 
-#ifdef CONFIG_FAULT_INJECTION
-#include <linux/fault-inject.h> /* XXX: fault-inject.h is broken */
 DECLARE_FAULT_ATTR(gt_reset_failure);
-#endif
 
 static struct xe_device *node_to_xe(struct drm_info_node *node)
 {
@@ -214,8 +212,5 @@ void xe_debugfs_register(struct xe_device *xe)
 	for_each_gt(gt, xe, id)
 		xe_gt_debugfs_register(gt);
 
-#ifdef CONFIG_FAULT_INJECTION
 	fault_create_debugfs_attr("fail_gt_reset", root, &gt_reset_failure);
-#endif
-
 }
diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
index 8b1a5027dcf2..ee138e9768a2 100644
--- a/drivers/gpu/drm/xe/xe_gt.h
+++ b/drivers/gpu/drm/xe/xe_gt.h
@@ -6,6 +6,8 @@
 #ifndef _XE_GT_H_
 #define _XE_GT_H_
 
+#include <linux/fault-inject.h>
+
 #include <drm/drm_util.h>
 
 #include "xe_device.h"
@@ -19,19 +21,11 @@
 
 #define CCS_MASK(gt) (((gt)->info.engine_mask & XE_HW_ENGINE_CCS_MASK) >> XE_HW_ENGINE_CCS0)
 
-#ifdef CONFIG_FAULT_INJECTION
-#include <linux/fault-inject.h> /* XXX: fault-inject.h is broken */
 extern struct fault_attr gt_reset_failure;
 static inline bool xe_fault_inject_gt_reset(void)
 {
 	return should_fail(&gt_reset_failure, 1);
 }
-#else
-static inline bool xe_fault_inject_gt_reset(void)
-{
-	return false;
-}
-#endif
 
 struct xe_gt *xe_gt_alloc(struct xe_tile *tile);
 int xe_gt_init_hwconfig(struct xe_gt *gt);
-- 
2.39.2

Re: [PATCH 3/3] drm/xe: clean up fault injection usage

[Thomas Hellström]

On Tue, 2024-08-13 at 15:12 +0300, Jani Nikula wrote:
> With the proper stubs in place in linux/fault-inject.h, we can remove
> a
> bunch of conditional compilation for CONFIG_FAULT_INJECTION=n.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> ---
>  drivers/gpu/drm/xe/xe_debugfs.c |  7 +------
>  drivers/gpu/drm/xe/xe_gt.h      | 10 ++--------
>  2 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
> b/drivers/gpu/drm/xe/xe_debugfs.c
> index 1011e5d281fa..b381bfb634f7 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -6,6 +6,7 @@
>  #include "xe_debugfs.h"
>  
>  #include <linux/debugfs.h>
> +#include <linux/fault-inject.h>
>  #include <linux/string_helpers.h>
>  
>  #include <drm/drm_debugfs.h>
> @@ -26,10 +27,7 @@
>  #include "xe_vm.h"
>  #endif
>  
> -#ifdef CONFIG_FAULT_INJECTION
> -#include <linux/fault-inject.h> /* XXX: fault-inject.h is broken */
>  DECLARE_FAULT_ATTR(gt_reset_failure);
> -#endif
>  
>  static struct xe_device *node_to_xe(struct drm_info_node *node)
>  {
> @@ -214,8 +212,5 @@ void xe_debugfs_register(struct xe_device *xe)
>  	for_each_gt(gt, xe, id)
>  		xe_gt_debugfs_register(gt);
>  
> -#ifdef CONFIG_FAULT_INJECTION
>  	fault_create_debugfs_attr("fail_gt_reset", root,
> &gt_reset_failure);
> -#endif
> -
>  }
> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> index 8b1a5027dcf2..ee138e9768a2 100644
> --- a/drivers/gpu/drm/xe/xe_gt.h
> +++ b/drivers/gpu/drm/xe/xe_gt.h
> @@ -6,6 +6,8 @@
>  #ifndef _XE_GT_H_
>  #define _XE_GT_H_
>  
> +#include <linux/fault-inject.h>
> +
>  #include <drm/drm_util.h>
>  
>  #include "xe_device.h"
> @@ -19,19 +21,11 @@
>  
>  #define CCS_MASK(gt) (((gt)->info.engine_mask &
> XE_HW_ENGINE_CCS_MASK) >> XE_HW_ENGINE_CCS0)
>  
> -#ifdef CONFIG_FAULT_INJECTION
> -#include <linux/fault-inject.h> /* XXX: fault-inject.h is broken */
>  extern struct fault_attr gt_reset_failure;
>  static inline bool xe_fault_inject_gt_reset(void)
>  {
>  	return should_fail(&gt_reset_failure, 1);
>  }
> -#else
> -static inline bool xe_fault_inject_gt_reset(void)
> -{
> -	return false;
> -}
> -#endif
>  
>  struct xe_gt *xe_gt_alloc(struct xe_tile *tile);
>  int xe_gt_init_hwconfig(struct xe_gt *gt);

Re: [PATCH 3/3] drm/xe: clean up fault injection usage

[Ghimiray, Himal Prasad]

On 13-08-2024 17:42, Jani Nikula wrote:
> With the proper stubs in place in linux/fault-inject.h, we can remove a
> bunch of conditional compilation for CONFIG_FAULT_INJECTION=n.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

I had previously raised the patch 
[https://lore.kernel.org/lkml/ZYBNDXoVO7LB_S0d@rdvivi-mobl4/T/] to 
address the header inclusion dependency in lklm.

Given that [Patch 1/3] in this series also makes function inclusions, 
such as should_fail, independent of configs, this seems to be an 
improved version.

Assuming Patch 1 is approved by the maintainers, this patch looks good 
to me.

Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>

> ---
>   drivers/gpu/drm/xe/xe_debugfs.c |  7 +------
>   drivers/gpu/drm/xe/xe_gt.h      | 10 ++--------
>   2 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index 1011e5d281fa..b381bfb634f7 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -6,6 +6,7 @@
>   #include "xe_debugfs.h"
>   
>   #include <linux/debugfs.h>
> +#include <linux/fault-inject.h>
>   #include <linux/string_helpers.h>
>   
>   #include <drm/drm_debugfs.h>
> @@ -26,10 +27,7 @@
>   #include "xe_vm.h"
>   #endif
>   
> -#ifdef CONFIG_FAULT_INJECTION
> -#include <linux/fault-inject.h> /* XXX: fault-inject.h is broken */
>   DECLARE_FAULT_ATTR(gt_reset_failure);
> -#endif
>   
>   static struct xe_device *node_to_xe(struct drm_info_node *node)
>   {
> @@ -214,8 +212,5 @@ void xe_debugfs_register(struct xe_device *xe)
>   	for_each_gt(gt, xe, id)
>   		xe_gt_debugfs_register(gt);
>   
> -#ifdef CONFIG_FAULT_INJECTION
>   	fault_create_debugfs_attr("fail_gt_reset", root, &gt_reset_failure);
> -#endif
> -
>   }
> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> index 8b1a5027dcf2..ee138e9768a2 100644
> --- a/drivers/gpu/drm/xe/xe_gt.h
> +++ b/drivers/gpu/drm/xe/xe_gt.h
> @@ -6,6 +6,8 @@
>   #ifndef _XE_GT_H_
>   #define _XE_GT_H_
>   
> +#include <linux/fault-inject.h>
> +
>   #include <drm/drm_util.h>
>   
>   #include "xe_device.h"
> @@ -19,19 +21,11 @@
>   
>   #define CCS_MASK(gt) (((gt)->info.engine_mask & XE_HW_ENGINE_CCS_MASK) >> XE_HW_ENGINE_CCS0)
>   
> -#ifdef CONFIG_FAULT_INJECTION
> -#include <linux/fault-inject.h> /* XXX: fault-inject.h is broken */
>   extern struct fault_attr gt_reset_failure;
>   static inline bool xe_fault_inject_gt_reset(void)
>   {
>   	return should_fail(&gt_reset_failure, 1);
>   }
> -#else
> -static inline bool xe_fault_inject_gt_reset(void)
> -{
> -	return false;
> -}
> -#endif
>   
>   struct xe_gt *xe_gt_alloc(struct xe_tile *tile);
>   int xe_gt_init_hwconfig(struct xe_gt *gt);

Re: [PATCH 1/3] fault-inject: improve build for CONFIG_FAULT_INJECTION=n

[Ghimiray, Himal Prasad]

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

Re: [PATCH 1/3] fault-inject: improve build for CONFIG_FAULT_INJECTION=n

[Andrew Morton]

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

Re: [PATCH 1/3] fault-inject: improve build for CONFIG_FAULT_INJECTION=n

[Jani Nikula]

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

-- 
Jani Nikula, Intel

Re: [PATCH 1/3] fault-inject: improve build for CONFIG_FAULT_INJECTION=n

[Andrew Morton]

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.