[PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()

[PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()

[José Roberto de Souza]

Add function to set a custom coredump timeout.

For Xe driver usage, current 5 minutes timeout may be too short for
users to search and understand what needs to be done to capture
coredump to report bugs.

We have plans to automate(distribute a udev script) it but at the end
will be up to distros and users to pack it so having a option to
increase the timeout is a safer option.

v2:
- replace dev_coredump_timeout_set() by dev_coredumpm_timeout() (Mukesh)

v3:
- make dev_coredumpm() static inline (Johannes)

v5:
- rename DEVCOREDUMP_TIMEOUT -> DEVCD_TIMEOUT to avoid redefinition
in include/net/bluetooth/coredump.h

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Mukesh Ojha <quic_mojha@quicinc.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
Acked-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/base/devcoredump.c  | 23 ++++++++--------
 include/linux/devcoredump.h | 54 ++++++++++++++++++++++++++++---------
 2 files changed, 54 insertions(+), 23 deletions(-)

diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
index 82aeb09b3d1b5..c795edad1b969 100644
--- a/drivers/base/devcoredump.c
+++ b/drivers/base/devcoredump.c
@@ -18,9 +18,6 @@ static struct class devcd_class;
 /* global disable flag, for security purposes */
 static bool devcd_disabled;
 
-/* if data isn't read by userspace after 5 minutes then delete it */
-#define DEVCD_TIMEOUT	(HZ * 60 * 5)
-
 struct devcd_entry {
 	struct device devcd_dev;
 	void *data;
@@ -328,7 +325,8 @@ void dev_coredump_put(struct device *dev)
 EXPORT_SYMBOL_GPL(dev_coredump_put);
 
 /**
- * dev_coredumpm - create device coredump with read/free methods
+ * dev_coredumpm_timeout - create device coredump with read/free methods with a
+ * custom timeout.
  * @dev: the struct device for the crashed device
  * @owner: the module that contains the read/free functions, use %THIS_MODULE
  * @data: data cookie for the @read/@free functions
@@ -336,17 +334,20 @@ EXPORT_SYMBOL_GPL(dev_coredump_put);
  * @gfp: allocation flags
  * @read: function to read from the given buffer
  * @free: function to free the given buffer
+ * @timeout: time in jiffies to remove coredump
  *
  * Creates a new device coredump for the given device. If a previous one hasn't
  * been read yet, the new coredump is discarded. The data lifetime is determined
  * by the device coredump framework and when it is no longer needed the @free
  * function will be called to free the data.
  */
-void dev_coredumpm(struct device *dev, struct module *owner,
-		   void *data, size_t datalen, gfp_t gfp,
-		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
-				   void *data, size_t datalen),
-		   void (*free)(void *data))
+void dev_coredumpm_timeout(struct device *dev, struct module *owner,
+			   void *data, size_t datalen, gfp_t gfp,
+			   ssize_t (*read)(char *buffer, loff_t offset,
+					   size_t count, void *data,
+					   size_t datalen),
+			   void (*free)(void *data),
+			   unsigned long timeout)
 {
 	static atomic_t devcd_count = ATOMIC_INIT(0);
 	struct devcd_entry *devcd;
@@ -403,7 +404,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
 	dev_set_uevent_suppress(&devcd->devcd_dev, false);
 	kobject_uevent(&devcd->devcd_dev.kobj, KOBJ_ADD);
 	INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
-	schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
+	schedule_delayed_work(&devcd->del_wk, timeout);
 	mutex_unlock(&devcd->mutex);
 	return;
  put_device:
@@ -414,7 +415,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
  free:
 	free(data);
 }
-EXPORT_SYMBOL_GPL(dev_coredumpm);
+EXPORT_SYMBOL_GPL(dev_coredumpm_timeout);
 
 /**
  * dev_coredumpsg - create device coredump that uses scatterlist as data
diff --git a/include/linux/devcoredump.h b/include/linux/devcoredump.h
index c8f7eb6cc1915..e3de1e545a4a5 100644
--- a/include/linux/devcoredump.h
+++ b/include/linux/devcoredump.h
@@ -12,6 +12,9 @@
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 
+/* if data isn't read by userspace after 5 minutes then delete it */
+#define DEVCD_TIMEOUT	(HZ * 60 * 5)
+
 /*
  * _devcd_free_sgtable - free all the memory of the given scatterlist table
  * (i.e. both pages and scatterlist instances)
@@ -50,16 +53,17 @@ static inline void _devcd_free_sgtable(struct scatterlist *table)
 	kfree(delete_iter);
 }
 
-
 #ifdef CONFIG_DEV_COREDUMP
 void dev_coredumpv(struct device *dev, void *data, size_t datalen,
 		   gfp_t gfp);
 
-void dev_coredumpm(struct device *dev, struct module *owner,
-		   void *data, size_t datalen, gfp_t gfp,
-		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
-				   void *data, size_t datalen),
-		   void (*free)(void *data));
+void dev_coredumpm_timeout(struct device *dev, struct module *owner,
+			   void *data, size_t datalen, gfp_t gfp,
+			   ssize_t (*read)(char *buffer, loff_t offset,
+					   size_t count, void *data,
+					   size_t datalen),
+			   void (*free)(void *data),
+			   unsigned long timeout);
 
 void dev_coredumpsg(struct device *dev, struct scatterlist *table,
 		    size_t datalen, gfp_t gfp);
@@ -72,12 +76,13 @@ static inline void dev_coredumpv(struct device *dev, void *data,
 	vfree(data);
 }
 
-static inline void
-dev_coredumpm(struct device *dev, struct module *owner,
-	      void *data, size_t datalen, gfp_t gfp,
-	      ssize_t (*read)(char *buffer, loff_t offset, size_t count,
-			      void *data, size_t datalen),
-	      void (*free)(void *data))
+void dev_coredumpm_timeout(struct device *dev, struct module *owner,
+			   void *data, size_t datalen, gfp_t gfp,
+			   ssize_t (*read)(char *buffer, loff_t offset,
+					   size_t count, void *data,
+					   size_t datalen),
+			   void (*free)(void *data),
+			   unsigned long timeout)
 {
 	free(data);
 }
@@ -92,4 +97,29 @@ static inline void dev_coredump_put(struct device *dev)
 }
 #endif /* CONFIG_DEV_COREDUMP */
 
+/**
+ * dev_coredumpm - create device coredump with read/free methods
+ * @dev: the struct device for the crashed device
+ * @owner: the module that contains the read/free functions, use %THIS_MODULE
+ * @data: data cookie for the @read/@free functions
+ * @datalen: length of the data
+ * @gfp: allocation flags
+ * @read: function to read from the given buffer
+ * @free: function to free the given buffer
+ *
+ * Creates a new device coredump for the given device. If a previous one hasn't
+ * been read yet, the new coredump is discarded. The data lifetime is determined
+ * by the device coredump framework and when it is no longer needed the @free
+ * function will be called to free the data.
+ */
+static inline void dev_coredumpm(struct device *dev, struct module *owner,
+				 void *data, size_t datalen, gfp_t gfp,
+				 ssize_t (*read)(char *buffer, loff_t offset, size_t count,
+						 void *data, size_t datalen),
+				void (*free)(void *data))
+{
+	dev_coredumpm_timeout(dev, owner, data, datalen, gfp, read, free,
+			      DEVCD_TIMEOUT);
+}
+
 #endif /* __DEVCOREDUMP_H */
-- 
2.45.2

[PATCH v5 2/2] drm/xe: Increase devcoredump timeout

[José Roberto de Souza]

5 minutes is too short for a regular user to search and understand
what he needs to do to report capture devcoredump and report a bug to
us, so here increasing this timeout to 1 hour.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
Acked-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/xe/xe_devcoredump.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
index d7f2d19a77c10..62c2b10fbf1d2 100644
--- a/drivers/gpu/drm/xe/xe_devcoredump.c
+++ b/drivers/gpu/drm/xe/xe_devcoredump.c
@@ -53,6 +53,9 @@
 
 #ifdef CONFIG_DEV_COREDUMP
 
+/* 1 hour timeout */
+#define XE_COREDUMP_TIMEOUT_JIFFIES (60 * 60 * HZ)
+
 static struct xe_device *coredump_to_xe(const struct xe_devcoredump *coredump)
 {
 	return container_of(coredump, struct xe_device, devcoredump);
@@ -247,8 +250,9 @@ void xe_devcoredump(struct xe_sched_job *job)
 	drm_info(&xe->drm, "Check your /sys/class/drm/card%d/device/devcoredump/data\n",
 		 xe->drm.primary->index);
 
-	dev_coredumpm(xe->drm.dev, THIS_MODULE, coredump, 0, GFP_KERNEL,
-		      xe_devcoredump_read, xe_devcoredump_free);
+	dev_coredumpm_timeout(xe->drm.dev, THIS_MODULE, coredump, 0, GFP_KERNEL,
+			      xe_devcoredump_read, xe_devcoredump_free,
+			      XE_COREDUMP_TIMEOUT_JIFFIES);
 }
 
 static void xe_driver_devcoredump_fini(void *arg)
-- 
2.45.2

Re: [PATCH v5 2/2] drm/xe: Increase devcoredump timeout

[Rodrigo Vivi]

On Mon, Jun 10, 2024 at 09:11:33AM -0700, José Roberto de Souza wrote:
> 5 minutes is too short for a regular user to search and understand
> what he needs to do to report capture devcoredump and report a bug to
> us, so here increasing this timeout to 1 hour.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Acked-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_devcoredump.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index d7f2d19a77c10..62c2b10fbf1d2 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> @@ -53,6 +53,9 @@
>  
>  #ifdef CONFIG_DEV_COREDUMP
>  
> +/* 1 hour timeout */
> +#define XE_COREDUMP_TIMEOUT_JIFFIES (60 * 60 * HZ)

o.O! 1h?!
we should likely already add a config option for that.
but anyway, let's move with that and adjust as we go.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> +
>  static struct xe_device *coredump_to_xe(const struct xe_devcoredump *coredump)
>  {
>  	return container_of(coredump, struct xe_device, devcoredump);
> @@ -247,8 +250,9 @@ void xe_devcoredump(struct xe_sched_job *job)
>  	drm_info(&xe->drm, "Check your /sys/class/drm/card%d/device/devcoredump/data\n",
>  		 xe->drm.primary->index);
>  
> -	dev_coredumpm(xe->drm.dev, THIS_MODULE, coredump, 0, GFP_KERNEL,
> -		      xe_devcoredump_read, xe_devcoredump_free);
> +	dev_coredumpm_timeout(xe->drm.dev, THIS_MODULE, coredump, 0, GFP_KERNEL,
> +			      xe_devcoredump_read, xe_devcoredump_free,
> +			      XE_COREDUMP_TIMEOUT_JIFFIES);
>  }
>  
>  static void xe_driver_devcoredump_fini(void *arg)
> -- 
> 2.45.2
> 

Re: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()

[Rodrigo Vivi]

On Mon, Jun 10, 2024 at 09:11:32AM -0700, José Roberto de Souza wrote:
> Add function to set a custom coredump timeout.
> 
> For Xe driver usage, current 5 minutes timeout may be too short for
> users to search and understand what needs to be done to capture
> coredump to report bugs.
> 
> We have plans to automate(distribute a udev script) it but at the end
> will be up to distros and users to pack it so having a option to
> increase the timeout is a safer option.
> 
> v2:
> - replace dev_coredump_timeout_set() by dev_coredumpm_timeout() (Mukesh)
> 
> v3:
> - make dev_coredumpm() static inline (Johannes)
> 
> v5:
> - rename DEVCOREDUMP_TIMEOUT -> DEVCD_TIMEOUT to avoid redefinition
> in include/net/bluetooth/coredump.h
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> Cc: Mukesh Ojha <quic_mojha@quicinc.com>
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Acked-by: Jonathan Cavitt <jonathan.cavitt@intel.com>

Jonathan, also ack to merge this through drm-next flow?

> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/base/devcoredump.c  | 23 ++++++++--------
>  include/linux/devcoredump.h | 54 ++++++++++++++++++++++++++++---------
>  2 files changed, 54 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
> index 82aeb09b3d1b5..c795edad1b969 100644
> --- a/drivers/base/devcoredump.c
> +++ b/drivers/base/devcoredump.c
> @@ -18,9 +18,6 @@ static struct class devcd_class;
>  /* global disable flag, for security purposes */
>  static bool devcd_disabled;
>  
> -/* if data isn't read by userspace after 5 minutes then delete it */
> -#define DEVCD_TIMEOUT	(HZ * 60 * 5)
> -
>  struct devcd_entry {
>  	struct device devcd_dev;
>  	void *data;
> @@ -328,7 +325,8 @@ void dev_coredump_put(struct device *dev)
>  EXPORT_SYMBOL_GPL(dev_coredump_put);
>  
>  /**
> - * dev_coredumpm - create device coredump with read/free methods
> + * dev_coredumpm_timeout - create device coredump with read/free methods with a
> + * custom timeout.
>   * @dev: the struct device for the crashed device
>   * @owner: the module that contains the read/free functions, use %THIS_MODULE
>   * @data: data cookie for the @read/@free functions
> @@ -336,17 +334,20 @@ EXPORT_SYMBOL_GPL(dev_coredump_put);
>   * @gfp: allocation flags
>   * @read: function to read from the given buffer
>   * @free: function to free the given buffer
> + * @timeout: time in jiffies to remove coredump
>   *
>   * Creates a new device coredump for the given device. If a previous one hasn't
>   * been read yet, the new coredump is discarded. The data lifetime is determined
>   * by the device coredump framework and when it is no longer needed the @free
>   * function will be called to free the data.
>   */
> -void dev_coredumpm(struct device *dev, struct module *owner,
> -		   void *data, size_t datalen, gfp_t gfp,
> -		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> -				   void *data, size_t datalen),
> -		   void (*free)(void *data))
> +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> +			   void *data, size_t datalen, gfp_t gfp,
> +			   ssize_t (*read)(char *buffer, loff_t offset,
> +					   size_t count, void *data,
> +					   size_t datalen),
> +			   void (*free)(void *data),
> +			   unsigned long timeout)
>  {
>  	static atomic_t devcd_count = ATOMIC_INIT(0);
>  	struct devcd_entry *devcd;
> @@ -403,7 +404,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
>  	dev_set_uevent_suppress(&devcd->devcd_dev, false);
>  	kobject_uevent(&devcd->devcd_dev.kobj, KOBJ_ADD);
>  	INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
> -	schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
> +	schedule_delayed_work(&devcd->del_wk, timeout);
>  	mutex_unlock(&devcd->mutex);
>  	return;
>   put_device:
> @@ -414,7 +415,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
>   free:
>  	free(data);
>  }
> -EXPORT_SYMBOL_GPL(dev_coredumpm);
> +EXPORT_SYMBOL_GPL(dev_coredumpm_timeout);
>  
>  /**
>   * dev_coredumpsg - create device coredump that uses scatterlist as data
> diff --git a/include/linux/devcoredump.h b/include/linux/devcoredump.h
> index c8f7eb6cc1915..e3de1e545a4a5 100644
> --- a/include/linux/devcoredump.h
> +++ b/include/linux/devcoredump.h
> @@ -12,6 +12,9 @@
>  #include <linux/scatterlist.h>
>  #include <linux/slab.h>
>  
> +/* if data isn't read by userspace after 5 minutes then delete it */
> +#define DEVCD_TIMEOUT	(HZ * 60 * 5)
> +
>  /*
>   * _devcd_free_sgtable - free all the memory of the given scatterlist table
>   * (i.e. both pages and scatterlist instances)
> @@ -50,16 +53,17 @@ static inline void _devcd_free_sgtable(struct scatterlist *table)
>  	kfree(delete_iter);
>  }
>  
> -
>  #ifdef CONFIG_DEV_COREDUMP
>  void dev_coredumpv(struct device *dev, void *data, size_t datalen,
>  		   gfp_t gfp);
>  
> -void dev_coredumpm(struct device *dev, struct module *owner,
> -		   void *data, size_t datalen, gfp_t gfp,
> -		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> -				   void *data, size_t datalen),
> -		   void (*free)(void *data));
> +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> +			   void *data, size_t datalen, gfp_t gfp,
> +			   ssize_t (*read)(char *buffer, loff_t offset,
> +					   size_t count, void *data,
> +					   size_t datalen),
> +			   void (*free)(void *data),
> +			   unsigned long timeout);
>  
>  void dev_coredumpsg(struct device *dev, struct scatterlist *table,
>  		    size_t datalen, gfp_t gfp);
> @@ -72,12 +76,13 @@ static inline void dev_coredumpv(struct device *dev, void *data,
>  	vfree(data);
>  }
>  
> -static inline void
> -dev_coredumpm(struct device *dev, struct module *owner,
> -	      void *data, size_t datalen, gfp_t gfp,
> -	      ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> -			      void *data, size_t datalen),
> -	      void (*free)(void *data))
> +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> +			   void *data, size_t datalen, gfp_t gfp,
> +			   ssize_t (*read)(char *buffer, loff_t offset,
> +					   size_t count, void *data,
> +					   size_t datalen),
> +			   void (*free)(void *data),
> +			   unsigned long timeout)
>  {
>  	free(data);
>  }
> @@ -92,4 +97,29 @@ static inline void dev_coredump_put(struct device *dev)
>  }
>  #endif /* CONFIG_DEV_COREDUMP */
>  
> +/**
> + * dev_coredumpm - create device coredump with read/free methods
> + * @dev: the struct device for the crashed device
> + * @owner: the module that contains the read/free functions, use %THIS_MODULE
> + * @data: data cookie for the @read/@free functions
> + * @datalen: length of the data
> + * @gfp: allocation flags
> + * @read: function to read from the given buffer
> + * @free: function to free the given buffer
> + *
> + * Creates a new device coredump for the given device. If a previous one hasn't
> + * been read yet, the new coredump is discarded. The data lifetime is determined
> + * by the device coredump framework and when it is no longer needed the @free
> + * function will be called to free the data.
> + */
> +static inline void dev_coredumpm(struct device *dev, struct module *owner,
> +				 void *data, size_t datalen, gfp_t gfp,
> +				 ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +						 void *data, size_t datalen),
> +				void (*free)(void *data))
> +{
> +	dev_coredumpm_timeout(dev, owner, data, datalen, gfp, read, free,
> +			      DEVCD_TIMEOUT);
> +}
> +
>  #endif /* __DEVCOREDUMP_H */
> -- 
> 2.45.2
> 

RE: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()

[Cavitt, Jonathan]

-----Original Message-----
From: Vivi, Rodrigo <rodrigo.vivi@intel.com> 
Sent: Monday, June 10, 2024 1:16 PM
To: Souza, Jose <jose.souza@intel.com>
Cc: linux-kernel@vger.kernel.org; intel-xe@lists.freedesktop.org; Mukesh Ojha <quic_mojha@quicinc.com>; Johannes Berg <johannes@sipsolutions.net>; Cavitt, Jonathan <jonathan.cavitt@intel.com>
Subject: Re: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()
> 
> On Mon, Jun 10, 2024 at 09:11:32AM -0700, José Roberto de Souza wrote:
> > Add function to set a custom coredump timeout.
> > 
> > For Xe driver usage, current 5 minutes timeout may be too short for
> > users to search and understand what needs to be done to capture
> > coredump to report bugs.
> > 
> > We have plans to automate(distribute a udev script) it but at the end
> > will be up to distros and users to pack it so having a option to
> > increase the timeout is a safer option.
> > 
> > v2:
> > - replace dev_coredump_timeout_set() by dev_coredumpm_timeout() (Mukesh)
> > 
> > v3:
> > - make dev_coredumpm() static inline (Johannes)
> > 
> > v5:
> > - rename DEVCOREDUMP_TIMEOUT -> DEVCD_TIMEOUT to avoid redefinition
> > in include/net/bluetooth/coredump.h
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> > Cc: Mukesh Ojha <quic_mojha@quicinc.com>
> > Cc: Johannes Berg <johannes@sipsolutions.net>
> > Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > Acked-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> 
> Jonathan, also ack to merge this through drm-next flow?

Ack clear for drm-next flow.  Actually, you can upgrade that to a
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
I provided the Ack back before I knew I was allowed to provide
Reviewed-bys, or that they were different from Acks even in my
case.  I trust that my past-self who originally reviewed this meant
for the Ack to be a stand-in as a non-committal RB.

-Jonathan Cavitt

> 
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/base/devcoredump.c  | 23 ++++++++--------
> >  include/linux/devcoredump.h | 54 ++++++++++++++++++++++++++++---------
> >  2 files changed, 54 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
> > index 82aeb09b3d1b5..c795edad1b969 100644
> > --- a/drivers/base/devcoredump.c
> > +++ b/drivers/base/devcoredump.c
> > @@ -18,9 +18,6 @@ static struct class devcd_class;
> >  /* global disable flag, for security purposes */
> >  static bool devcd_disabled;
> >  
> > -/* if data isn't read by userspace after 5 minutes then delete it */
> > -#define DEVCD_TIMEOUT	(HZ * 60 * 5)
> > -
> >  struct devcd_entry {
> >  	struct device devcd_dev;
> >  	void *data;
> > @@ -328,7 +325,8 @@ void dev_coredump_put(struct device *dev)
> >  EXPORT_SYMBOL_GPL(dev_coredump_put);
> >  
> >  /**
> > - * dev_coredumpm - create device coredump with read/free methods
> > + * dev_coredumpm_timeout - create device coredump with read/free methods with a
> > + * custom timeout.
> >   * @dev: the struct device for the crashed device
> >   * @owner: the module that contains the read/free functions, use %THIS_MODULE
> >   * @data: data cookie for the @read/@free functions
> > @@ -336,17 +334,20 @@ EXPORT_SYMBOL_GPL(dev_coredump_put);
> >   * @gfp: allocation flags
> >   * @read: function to read from the given buffer
> >   * @free: function to free the given buffer
> > + * @timeout: time in jiffies to remove coredump
> >   *
> >   * Creates a new device coredump for the given device. If a previous one hasn't
> >   * been read yet, the new coredump is discarded. The data lifetime is determined
> >   * by the device coredump framework and when it is no longer needed the @free
> >   * function will be called to free the data.
> >   */
> > -void dev_coredumpm(struct device *dev, struct module *owner,
> > -		   void *data, size_t datalen, gfp_t gfp,
> > -		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > -				   void *data, size_t datalen),
> > -		   void (*free)(void *data))
> > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > +			   void *data, size_t datalen, gfp_t gfp,
> > +			   ssize_t (*read)(char *buffer, loff_t offset,
> > +					   size_t count, void *data,
> > +					   size_t datalen),
> > +			   void (*free)(void *data),
> > +			   unsigned long timeout)
> >  {
> >  	static atomic_t devcd_count = ATOMIC_INIT(0);
> >  	struct devcd_entry *devcd;
> > @@ -403,7 +404,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> >  	dev_set_uevent_suppress(&devcd->devcd_dev, false);
> >  	kobject_uevent(&devcd->devcd_dev.kobj, KOBJ_ADD);
> >  	INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
> > -	schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
> > +	schedule_delayed_work(&devcd->del_wk, timeout);
> >  	mutex_unlock(&devcd->mutex);
> >  	return;
> >   put_device:
> > @@ -414,7 +415,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> >   free:
> >  	free(data);
> >  }
> > -EXPORT_SYMBOL_GPL(dev_coredumpm);
> > +EXPORT_SYMBOL_GPL(dev_coredumpm_timeout);
> >  
> >  /**
> >   * dev_coredumpsg - create device coredump that uses scatterlist as data
> > diff --git a/include/linux/devcoredump.h b/include/linux/devcoredump.h
> > index c8f7eb6cc1915..e3de1e545a4a5 100644
> > --- a/include/linux/devcoredump.h
> > +++ b/include/linux/devcoredump.h
> > @@ -12,6 +12,9 @@
> >  #include <linux/scatterlist.h>
> >  #include <linux/slab.h>
> >  
> > +/* if data isn't read by userspace after 5 minutes then delete it */
> > +#define DEVCD_TIMEOUT	(HZ * 60 * 5)
> > +
> >  /*
> >   * _devcd_free_sgtable - free all the memory of the given scatterlist table
> >   * (i.e. both pages and scatterlist instances)
> > @@ -50,16 +53,17 @@ static inline void _devcd_free_sgtable(struct scatterlist *table)
> >  	kfree(delete_iter);
> >  }
> >  
> > -
> >  #ifdef CONFIG_DEV_COREDUMP
> >  void dev_coredumpv(struct device *dev, void *data, size_t datalen,
> >  		   gfp_t gfp);
> >  
> > -void dev_coredumpm(struct device *dev, struct module *owner,
> > -		   void *data, size_t datalen, gfp_t gfp,
> > -		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > -				   void *data, size_t datalen),
> > -		   void (*free)(void *data));
> > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > +			   void *data, size_t datalen, gfp_t gfp,
> > +			   ssize_t (*read)(char *buffer, loff_t offset,
> > +					   size_t count, void *data,
> > +					   size_t datalen),
> > +			   void (*free)(void *data),
> > +			   unsigned long timeout);
> >  
> >  void dev_coredumpsg(struct device *dev, struct scatterlist *table,
> >  		    size_t datalen, gfp_t gfp);
> > @@ -72,12 +76,13 @@ static inline void dev_coredumpv(struct device *dev, void *data,
> >  	vfree(data);
> >  }
> >  
> > -static inline void
> > -dev_coredumpm(struct device *dev, struct module *owner,
> > -	      void *data, size_t datalen, gfp_t gfp,
> > -	      ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > -			      void *data, size_t datalen),
> > -	      void (*free)(void *data))
> > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > +			   void *data, size_t datalen, gfp_t gfp,
> > +			   ssize_t (*read)(char *buffer, loff_t offset,
> > +					   size_t count, void *data,
> > +					   size_t datalen),
> > +			   void (*free)(void *data),
> > +			   unsigned long timeout)
> >  {
> >  	free(data);
> >  }
> > @@ -92,4 +97,29 @@ static inline void dev_coredump_put(struct device *dev)
> >  }
> >  #endif /* CONFIG_DEV_COREDUMP */
> >  
> > +/**
> > + * dev_coredumpm - create device coredump with read/free methods
> > + * @dev: the struct device for the crashed device
> > + * @owner: the module that contains the read/free functions, use %THIS_MODULE
> > + * @data: data cookie for the @read/@free functions
> > + * @datalen: length of the data
> > + * @gfp: allocation flags
> > + * @read: function to read from the given buffer
> > + * @free: function to free the given buffer
> > + *
> > + * Creates a new device coredump for the given device. If a previous one hasn't
> > + * been read yet, the new coredump is discarded. The data lifetime is determined
> > + * by the device coredump framework and when it is no longer needed the @free
> > + * function will be called to free the data.
> > + */
> > +static inline void dev_coredumpm(struct device *dev, struct module *owner,
> > +				 void *data, size_t datalen, gfp_t gfp,
> > +				 ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > +						 void *data, size_t datalen),
> > +				void (*free)(void *data))
> > +{
> > +	dev_coredumpm_timeout(dev, owner, data, datalen, gfp, read, free,
> > +			      DEVCD_TIMEOUT);
> > +}
> > +
> >  #endif /* __DEVCOREDUMP_H */
> > -- 
> > 2.45.2
> > 
> 

Re: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()

[Souza, Jose]

On Mon, 2024-06-10 at 20:42 +0000, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com> 
> Sent: Monday, June 10, 2024 1:16 PM
> To: Souza, Jose <jose.souza@intel.com>
> Cc: linux-kernel@vger.kernel.org; intel-xe@lists.freedesktop.org; Mukesh Ojha <quic_mojha@quicinc.com>; Johannes Berg <johannes@sipsolutions.net>; Cavitt, Jonathan <jonathan.cavitt@intel.com>
> Subject: Re: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()
> > 
> > On Mon, Jun 10, 2024 at 09:11:32AM -0700, José Roberto de Souza wrote:
> > > Add function to set a custom coredump timeout.
> > > 
> > > For Xe driver usage, current 5 minutes timeout may be too short for
> > > users to search and understand what needs to be done to capture
> > > coredump to report bugs.
> > > 
> > > We have plans to automate(distribute a udev script) it but at the end
> > > will be up to distros and users to pack it so having a option to
> > > increase the timeout is a safer option.
> > > 
> > > v2:
> > > - replace dev_coredump_timeout_set() by dev_coredumpm_timeout() (Mukesh)
> > > 
> > > v3:
> > > - make dev_coredumpm() static inline (Johannes)
> > > 
> > > v5:
> > > - rename DEVCOREDUMP_TIMEOUT -> DEVCD_TIMEOUT to avoid redefinition
> > > in include/net/bluetooth/coredump.h
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > > Cc: Mukesh Ojha <quic_mojha@quicinc.com>
> > > Cc: Johannes Berg <johannes@sipsolutions.net>
> > > Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > > Acked-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > 
> > Jonathan, also ack to merge this through drm-next flow?
> 
> Ack clear for drm-next flow.  Actually, you can upgrade that to a
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> I provided the Ack back before I knew I was allowed to provide
> Reviewed-bys, or that they were different from Acks even in my
> case.  I trust that my past-self who originally reviewed this meant
> for the Ack to be a stand-in as a non-committal RB.


Thanks you both but I think we need wait for an ack from Johannes or Mukesh to merge this devcoredump patch.
On my other devcoredump patch already merged we also ask from a Greg's ack to merge it through drm-next.


> 
> -Jonathan Cavitt
> 
> > 
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/base/devcoredump.c  | 23 ++++++++--------
> > >  include/linux/devcoredump.h | 54 ++++++++++++++++++++++++++++---------
> > >  2 files changed, 54 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
> > > index 82aeb09b3d1b5..c795edad1b969 100644
> > > --- a/drivers/base/devcoredump.c
> > > +++ b/drivers/base/devcoredump.c
> > > @@ -18,9 +18,6 @@ static struct class devcd_class;
> > >  /* global disable flag, for security purposes */
> > >  static bool devcd_disabled;
> > >  
> > > -/* if data isn't read by userspace after 5 minutes then delete it */
> > > -#define DEVCD_TIMEOUT	(HZ * 60 * 5)
> > > -
> > >  struct devcd_entry {
> > >  	struct device devcd_dev;
> > >  	void *data;
> > > @@ -328,7 +325,8 @@ void dev_coredump_put(struct device *dev)
> > >  EXPORT_SYMBOL_GPL(dev_coredump_put);
> > >  
> > >  /**
> > > - * dev_coredumpm - create device coredump with read/free methods
> > > + * dev_coredumpm_timeout - create device coredump with read/free methods with a
> > > + * custom timeout.
> > >   * @dev: the struct device for the crashed device
> > >   * @owner: the module that contains the read/free functions, use %THIS_MODULE
> > >   * @data: data cookie for the @read/@free functions
> > > @@ -336,17 +334,20 @@ EXPORT_SYMBOL_GPL(dev_coredump_put);
> > >   * @gfp: allocation flags
> > >   * @read: function to read from the given buffer
> > >   * @free: function to free the given buffer
> > > + * @timeout: time in jiffies to remove coredump
> > >   *
> > >   * Creates a new device coredump for the given device. If a previous one hasn't
> > >   * been read yet, the new coredump is discarded. The data lifetime is determined
> > >   * by the device coredump framework and when it is no longer needed the @free
> > >   * function will be called to free the data.
> > >   */
> > > -void dev_coredumpm(struct device *dev, struct module *owner,
> > > -		   void *data, size_t datalen, gfp_t gfp,
> > > -		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > > -				   void *data, size_t datalen),
> > > -		   void (*free)(void *data))
> > > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > > +			   void *data, size_t datalen, gfp_t gfp,
> > > +			   ssize_t (*read)(char *buffer, loff_t offset,
> > > +					   size_t count, void *data,
> > > +					   size_t datalen),
> > > +			   void (*free)(void *data),
> > > +			   unsigned long timeout)
> > >  {
> > >  	static atomic_t devcd_count = ATOMIC_INIT(0);
> > >  	struct devcd_entry *devcd;
> > > @@ -403,7 +404,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> > >  	dev_set_uevent_suppress(&devcd->devcd_dev, false);
> > >  	kobject_uevent(&devcd->devcd_dev.kobj, KOBJ_ADD);
> > >  	INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
> > > -	schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
> > > +	schedule_delayed_work(&devcd->del_wk, timeout);
> > >  	mutex_unlock(&devcd->mutex);
> > >  	return;
> > >   put_device:
> > > @@ -414,7 +415,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> > >   free:
> > >  	free(data);
> > >  }
> > > -EXPORT_SYMBOL_GPL(dev_coredumpm);
> > > +EXPORT_SYMBOL_GPL(dev_coredumpm_timeout);
> > >  
> > >  /**
> > >   * dev_coredumpsg - create device coredump that uses scatterlist as data
> > > diff --git a/include/linux/devcoredump.h b/include/linux/devcoredump.h
> > > index c8f7eb6cc1915..e3de1e545a4a5 100644
> > > --- a/include/linux/devcoredump.h
> > > +++ b/include/linux/devcoredump.h
> > > @@ -12,6 +12,9 @@
> > >  #include <linux/scatterlist.h>
> > >  #include <linux/slab.h>
> > >  
> > > +/* if data isn't read by userspace after 5 minutes then delete it */
> > > +#define DEVCD_TIMEOUT	(HZ * 60 * 5)
> > > +
> > >  /*
> > >   * _devcd_free_sgtable - free all the memory of the given scatterlist table
> > >   * (i.e. both pages and scatterlist instances)
> > > @@ -50,16 +53,17 @@ static inline void _devcd_free_sgtable(struct scatterlist *table)
> > >  	kfree(delete_iter);
> > >  }
> > >  
> > > -
> > >  #ifdef CONFIG_DEV_COREDUMP
> > >  void dev_coredumpv(struct device *dev, void *data, size_t datalen,
> > >  		   gfp_t gfp);
> > >  
> > > -void dev_coredumpm(struct device *dev, struct module *owner,
> > > -		   void *data, size_t datalen, gfp_t gfp,
> > > -		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > > -				   void *data, size_t datalen),
> > > -		   void (*free)(void *data));
> > > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > > +			   void *data, size_t datalen, gfp_t gfp,
> > > +			   ssize_t (*read)(char *buffer, loff_t offset,
> > > +					   size_t count, void *data,
> > > +					   size_t datalen),
> > > +			   void (*free)(void *data),
> > > +			   unsigned long timeout);
> > >  
> > >  void dev_coredumpsg(struct device *dev, struct scatterlist *table,
> > >  		    size_t datalen, gfp_t gfp);
> > > @@ -72,12 +76,13 @@ static inline void dev_coredumpv(struct device *dev, void *data,
> > >  	vfree(data);
> > >  }
> > >  
> > > -static inline void
> > > -dev_coredumpm(struct device *dev, struct module *owner,
> > > -	      void *data, size_t datalen, gfp_t gfp,
> > > -	      ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > > -			      void *data, size_t datalen),
> > > -	      void (*free)(void *data))
> > > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > > +			   void *data, size_t datalen, gfp_t gfp,
> > > +			   ssize_t (*read)(char *buffer, loff_t offset,
> > > +					   size_t count, void *data,
> > > +					   size_t datalen),
> > > +			   void (*free)(void *data),
> > > +			   unsigned long timeout)
> > >  {
> > >  	free(data);
> > >  }
> > > @@ -92,4 +97,29 @@ static inline void dev_coredump_put(struct device *dev)
> > >  }
> > >  #endif /* CONFIG_DEV_COREDUMP */
> > >  
> > > +/**
> > > + * dev_coredumpm - create device coredump with read/free methods
> > > + * @dev: the struct device for the crashed device
> > > + * @owner: the module that contains the read/free functions, use %THIS_MODULE
> > > + * @data: data cookie for the @read/@free functions
> > > + * @datalen: length of the data
> > > + * @gfp: allocation flags
> > > + * @read: function to read from the given buffer
> > > + * @free: function to free the given buffer
> > > + *
> > > + * Creates a new device coredump for the given device. If a previous one hasn't
> > > + * been read yet, the new coredump is discarded. The data lifetime is determined
> > > + * by the device coredump framework and when it is no longer needed the @free
> > > + * function will be called to free the data.
> > > + */
> > > +static inline void dev_coredumpm(struct device *dev, struct module *owner,
> > > +				 void *data, size_t datalen, gfp_t gfp,
> > > +				 ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > > +						 void *data, size_t datalen),
> > > +				void (*free)(void *data))
> > > +{
> > > +	dev_coredumpm_timeout(dev, owner, data, datalen, gfp, read, free,
> > > +			      DEVCD_TIMEOUT);
> > > +}
> > > +
> > >  #endif /* __DEVCOREDUMP_H */
> > > -- 
> > > 2.45.2
> > > 
> > 

Re: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()

[Rodrigo Vivi]

On Mon, Jun 10, 2024 at 04:52:11PM -0400, Souza, Jose wrote:
> On Mon, 2024-06-10 at 20:42 +0000, Cavitt, Jonathan wrote:
> > -----Original Message-----
> > From: Vivi, Rodrigo <rodrigo.vivi@intel.com> 
> > Sent: Monday, June 10, 2024 1:16 PM
> > To: Souza, Jose <jose.souza@intel.com>
> > Cc: linux-kernel@vger.kernel.org; intel-xe@lists.freedesktop.org; Mukesh Ojha <quic_mojha@quicinc.com>; Johannes Berg <johannes@sipsolutions.net>; Cavitt, Jonathan <jonathan.cavitt@intel.com>
> > Subject: Re: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()
> > > 
> > > On Mon, Jun 10, 2024 at 09:11:32AM -0700, José Roberto de Souza wrote:
> > > > Add function to set a custom coredump timeout.
> > > > 
> > > > For Xe driver usage, current 5 minutes timeout may be too short for
> > > > users to search and understand what needs to be done to capture
> > > > coredump to report bugs.
> > > > 
> > > > We have plans to automate(distribute a udev script) it but at the end
> > > > will be up to distros and users to pack it so having a option to
> > > > increase the timeout is a safer option.
> > > > 
> > > > v2:
> > > > - replace dev_coredump_timeout_set() by dev_coredumpm_timeout() (Mukesh)
> > > > 
> > > > v3:
> > > > - make dev_coredumpm() static inline (Johannes)
> > > > 
> > > > v5:
> > > > - rename DEVCOREDUMP_TIMEOUT -> DEVCD_TIMEOUT to avoid redefinition
> > > > in include/net/bluetooth/coredump.h
> > > > 
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > 
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > 
> > > > Cc: Mukesh Ojha <quic_mojha@quicinc.com>
> > > > Cc: Johannes Berg <johannes@sipsolutions.net>
> > > > Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > > > Acked-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > > 
> > > Jonathan, also ack to merge this through drm-next flow?
> > 
> > Ack clear for drm-next flow.  Actually, you can upgrade that to a
> > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > I provided the Ack back before I knew I was allowed to provide
> > Reviewed-bys, or that they were different from Acks even in my
> > case.  I trust that my past-self who originally reviewed this meant
> > for the Ack to be a stand-in as a non-committal RB.
> 
> 
> Thanks you both but I think we need wait for an ack from Johannes or Mukesh to merge this devcoredump patch.
> On my other devcoredump patch already merged we also ask from a Greg's ack to merge it through drm-next.

My bad. I'm sorry.
Of course I meant to ask ack from Johannes there. I though he had acked this
approach already.

> 
> 
> > 
> > -Jonathan Cavitt
> > 
> > > 
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  drivers/base/devcoredump.c  | 23 ++++++++--------
> > > >  include/linux/devcoredump.h | 54 ++++++++++++++++++++++++++++---------
> > > >  2 files changed, 54 insertions(+), 23 deletions(-)
> > > > 
> > > > diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
> > > > index 82aeb09b3d1b5..c795edad1b969 100644
> > > > --- a/drivers/base/devcoredump.c
> > > > +++ b/drivers/base/devcoredump.c
> > > > @@ -18,9 +18,6 @@ static struct class devcd_class;
> > > >  /* global disable flag, for security purposes */
> > > >  static bool devcd_disabled;
> > > >  
> > > > -/* if data isn't read by userspace after 5 minutes then delete it */
> > > > -#define DEVCD_TIMEOUT	(HZ * 60 * 5)
> > > > -
> > > >  struct devcd_entry {
> > > >  	struct device devcd_dev;
> > > >  	void *data;
> > > > @@ -328,7 +325,8 @@ void dev_coredump_put(struct device *dev)
> > > >  EXPORT_SYMBOL_GPL(dev_coredump_put);
> > > >  
> > > >  /**
> > > > - * dev_coredumpm - create device coredump with read/free methods
> > > > + * dev_coredumpm_timeout - create device coredump with read/free methods with a
> > > > + * custom timeout.
> > > >   * @dev: the struct device for the crashed device
> > > >   * @owner: the module that contains the read/free functions, use %THIS_MODULE
> > > >   * @data: data cookie for the @read/@free functions
> > > > @@ -336,17 +334,20 @@ EXPORT_SYMBOL_GPL(dev_coredump_put);
> > > >   * @gfp: allocation flags
> > > >   * @read: function to read from the given buffer
> > > >   * @free: function to free the given buffer
> > > > + * @timeout: time in jiffies to remove coredump
> > > >   *
> > > >   * Creates a new device coredump for the given device. If a previous one hasn't
> > > >   * been read yet, the new coredump is discarded. The data lifetime is determined
> > > >   * by the device coredump framework and when it is no longer needed the @free
> > > >   * function will be called to free the data.
> > > >   */
> > > > -void dev_coredumpm(struct device *dev, struct module *owner,
> > > > -		   void *data, size_t datalen, gfp_t gfp,
> > > > -		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > > > -				   void *data, size_t datalen),
> > > > -		   void (*free)(void *data))
> > > > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > > > +			   void *data, size_t datalen, gfp_t gfp,
> > > > +			   ssize_t (*read)(char *buffer, loff_t offset,
> > > > +					   size_t count, void *data,
> > > > +					   size_t datalen),
> > > > +			   void (*free)(void *data),
> > > > +			   unsigned long timeout)
> > > >  {
> > > >  	static atomic_t devcd_count = ATOMIC_INIT(0);
> > > >  	struct devcd_entry *devcd;
> > > > @@ -403,7 +404,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> > > >  	dev_set_uevent_suppress(&devcd->devcd_dev, false);
> > > >  	kobject_uevent(&devcd->devcd_dev.kobj, KOBJ_ADD);
> > > >  	INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
> > > > -	schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
> > > > +	schedule_delayed_work(&devcd->del_wk, timeout);
> > > >  	mutex_unlock(&devcd->mutex);
> > > >  	return;
> > > >   put_device:
> > > > @@ -414,7 +415,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> > > >   free:
> > > >  	free(data);
> > > >  }
> > > > -EXPORT_SYMBOL_GPL(dev_coredumpm);
> > > > +EXPORT_SYMBOL_GPL(dev_coredumpm_timeout);
> > > >  
> > > >  /**
> > > >   * dev_coredumpsg - create device coredump that uses scatterlist as data
> > > > diff --git a/include/linux/devcoredump.h b/include/linux/devcoredump.h
> > > > index c8f7eb6cc1915..e3de1e545a4a5 100644
> > > > --- a/include/linux/devcoredump.h
> > > > +++ b/include/linux/devcoredump.h
> > > > @@ -12,6 +12,9 @@
> > > >  #include <linux/scatterlist.h>
> > > >  #include <linux/slab.h>
> > > >  
> > > > +/* if data isn't read by userspace after 5 minutes then delete it */
> > > > +#define DEVCD_TIMEOUT	(HZ * 60 * 5)
> > > > +
> > > >  /*
> > > >   * _devcd_free_sgtable - free all the memory of the given scatterlist table
> > > >   * (i.e. both pages and scatterlist instances)
> > > > @@ -50,16 +53,17 @@ static inline void _devcd_free_sgtable(struct scatterlist *table)
> > > >  	kfree(delete_iter);
> > > >  }
> > > >  
> > > > -
> > > >  #ifdef CONFIG_DEV_COREDUMP
> > > >  void dev_coredumpv(struct device *dev, void *data, size_t datalen,
> > > >  		   gfp_t gfp);
> > > >  
> > > > -void dev_coredumpm(struct device *dev, struct module *owner,
> > > > -		   void *data, size_t datalen, gfp_t gfp,
> > > > -		   ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > > > -				   void *data, size_t datalen),
> > > > -		   void (*free)(void *data));
> > > > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > > > +			   void *data, size_t datalen, gfp_t gfp,
> > > > +			   ssize_t (*read)(char *buffer, loff_t offset,
> > > > +					   size_t count, void *data,
> > > > +					   size_t datalen),
> > > > +			   void (*free)(void *data),
> > > > +			   unsigned long timeout);
> > > >  
> > > >  void dev_coredumpsg(struct device *dev, struct scatterlist *table,
> > > >  		    size_t datalen, gfp_t gfp);
> > > > @@ -72,12 +76,13 @@ static inline void dev_coredumpv(struct device *dev, void *data,
> > > >  	vfree(data);
> > > >  }
> > > >  
> > > > -static inline void
> > > > -dev_coredumpm(struct device *dev, struct module *owner,
> > > > -	      void *data, size_t datalen, gfp_t gfp,
> > > > -	      ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > > > -			      void *data, size_t datalen),
> > > > -	      void (*free)(void *data))
> > > > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > > > +			   void *data, size_t datalen, gfp_t gfp,
> > > > +			   ssize_t (*read)(char *buffer, loff_t offset,
> > > > +					   size_t count, void *data,
> > > > +					   size_t datalen),
> > > > +			   void (*free)(void *data),
> > > > +			   unsigned long timeout)
> > > >  {
> > > >  	free(data);
> > > >  }
> > > > @@ -92,4 +97,29 @@ static inline void dev_coredump_put(struct device *dev)
> > > >  }
> > > >  #endif /* CONFIG_DEV_COREDUMP */
> > > >  
> > > > +/**
> > > > + * dev_coredumpm - create device coredump with read/free methods
> > > > + * @dev: the struct device for the crashed device
> > > > + * @owner: the module that contains the read/free functions, use %THIS_MODULE
> > > > + * @data: data cookie for the @read/@free functions
> > > > + * @datalen: length of the data
> > > > + * @gfp: allocation flags
> > > > + * @read: function to read from the given buffer
> > > > + * @free: function to free the given buffer
> > > > + *
> > > > + * Creates a new device coredump for the given device. If a previous one hasn't
> > > > + * been read yet, the new coredump is discarded. The data lifetime is determined
> > > > + * by the device coredump framework and when it is no longer needed the @free
> > > > + * function will be called to free the data.
> > > > + */
> > > > +static inline void dev_coredumpm(struct device *dev, struct module *owner,
> > > > +				 void *data, size_t datalen, gfp_t gfp,
> > > > +				 ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > > > +						 void *data, size_t datalen),
> > > > +				void (*free)(void *data))
> > > > +{
> > > > +	dev_coredumpm_timeout(dev, owner, data, datalen, gfp, read, free,
> > > > +			      DEVCD_TIMEOUT);
> > > > +}
> > > > +
> > > >  #endif /* __DEVCOREDUMP_H */
> > > > -- 
> > > > 2.45.2
> > > > 
> > > 
> 

Re: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()

[kernel test robot]

Hi José,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-xe/drm-xe-next]
[also build test ERROR on wireless/main linus/master v6.10-rc3 next-20240607]
[cannot apply to driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus wireless-next/main]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jos-Roberto-de-Souza/drm-xe-Increase-devcoredump-timeout/20240611-001400
base:   https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next
patch link:    https://lore.kernel.org/r/20240610161133.156566-1-jose.souza%40intel.com
patch subject: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()
config: i386-buildonly-randconfig-004-20240611 (https://download.01.org/0day-ci/archive/20240611/202406111410.gmDjiFQR-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240611/202406111410.gmDjiFQR-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406111410.gmDjiFQR-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: sound/soc/intel/avs/apl.o: in function `dev_coredumpm_timeout':
>> apl.c:(.text+0x64c): multiple definition of `dev_coredumpm_timeout'; sound/soc/intel/avs/skl.o:skl.c:(.text+0x2d0): first defined here
--
   In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c:26:0:
>> include/linux/devcoredump.h:79:6: error: no previous prototype for 'dev_coredumpm_timeout' [-Werror=missing-prototypes]
    void dev_coredumpm_timeout(struct device *dev, struct module *owner,
         ^~~~~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()

[kernel test robot]

Hi José,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-xe/drm-xe-next]
[also build test ERROR on wireless-next/main wireless/main linus/master v6.10-rc3 next-20240611]
[cannot apply to driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jos-Roberto-de-Souza/drm-xe-Increase-devcoredump-timeout/20240611-001400
base:   https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next
patch link:    https://lore.kernel.org/r/20240610161133.156566-1-jose.souza%40intel.com
patch subject: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()
config: i386-randconfig-141-20240611 (https://download.01.org/0day-ci/archive/20240611/202406111852.SdIM9NWt-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240611/202406111852.SdIM9NWt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406111852.SdIM9NWt-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/xe/xe_devcoredump.c:9:
>> include/linux/devcoredump.h:79:6: error: no previous prototype for function 'dev_coredumpm_timeout' [-Werror,-Wmissing-prototypes]
      79 | void dev_coredumpm_timeout(struct device *dev, struct module *owner,
         |      ^
   include/linux/devcoredump.h:79:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
      79 | void dev_coredumpm_timeout(struct device *dev, struct module *owner,
         | ^
         | static 
   1 error generated.


vim +/dev_coredumpm_timeout +79 include/linux/devcoredump.h

    78	
  > 79	void dev_coredumpm_timeout(struct device *dev, struct module *owner,
    80				   void *data, size_t datalen, gfp_t gfp,
    81				   ssize_t (*read)(char *buffer, loff_t offset,
    82						   size_t count, void *data,
    83						   size_t datalen),
    84				   void (*free)(void *data),
    85				   unsigned long timeout)
    86	{
    87		free(data);
    88	}
    89	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki