[PATCH 0/2] sysfs: allow user-space request for devcoredump

[PATCH 0/2] sysfs: allow user-space request for devcoredump

[Arend van Spriel]

From: Arend van Spriel <aspriel@gmail.com>

Since commit 833c95456a70 ("device coredump: add new device coredump class")
device drivers have a unified way to provide binary data obtained from a
failing_device to user-space. However, there may be use-cases in which the
driver has no reason to obtain the data, but user-space wants to initiate
it. This adds a coredump device attribute in sysfs when the driver bound to
the device supports the newly added coredump driver callback.

These patches apply to the driver-core-next branch of the driver-core
repository.

Arend van Spriel (2):
  sysfs: add attribute specification for /sysfs/devices/.../coredump
  drivers: base: add coredump driver ops

 Documentation/ABI/testing/sysfs-devices-coredump | 10 ++++++
 drivers/base/dd.c                                | 40 +++++++++++++++++++-----
 include/linux/device.h                           |  2 +-
 3 files changed, 44 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-coredump

-- 
1.9.1

[PATCH 1/2] sysfs: add attribute specification for /sysfs/devices/.../coredump

[Arend van Spriel]

From: Arend van Spriel <aspriel@gmail.com>

This patch adds the specification for /sysfs/devices/.../coredump
which allows user-space to trigger a device coredump obtaining
binary data from the device for (fault) analysis. It relies on
CONFIG_DEV_COREDUMP being enabled.

Signed-off-by: Arend van Spriel <aspriel@gmail.com>
---
 Documentation/ABI/testing/sysfs-devices-coredump | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-coredump

diff --git a/Documentation/ABI/testing/sysfs-devices-coredump b/Documentation/ABI/testing/sysfs-devices-coredump
new file mode 100644
index 0000000..5989255
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-coredump
@@ -0,0 +1,10 @@
+What:		/sys/devices/.../coredump
+Date:		December 2017
+Contact:	Arend van Spriel <arend.vanspriel@broadcom.com>
+Description:
+		The /sys/devices/.../coredump attribute is only present when the
+		device is bound to a driver, which provides the .coredump()
+		callback. The attribute is write only. Anything written to this
+		file will trigger the .coredump() callback.
+
+		Available when CONFIG_DEV_COREDUMP is enabled.
-- 
1.9.1

[PATCH 2/2] drivers: base: add coredump driver ops

[Arend van Spriel]

From: Arend van Spriel <aspriel@gmail.com>

This adds the coredump driver operation. When the driver defines it
a coredump file is added in the sysfs folder of the device upon
driver binding. The file is removed when the driver is unbound.
User-space can trigger a coredump for this device by echo'ing to
the coredump file.

Signed-off-by: Arend van Spriel <aspriel@gmail.com>
---
 drivers/base/dd.c      | 40 +++++++++++++++++++++++++++++++++-------
 include/linux/device.h |  2 +-
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 533c82f..de6fd09 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -288,6 +288,18 @@ static void driver_bound(struct device *dev)
 	kobject_uevent(&dev->kobj, KOBJ_BIND);
 }
 
+static ssize_t coredump_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	device_lock(dev);
+	if (dev->driver->coredump)
+		dev->driver->coredump(dev);
+	device_unlock(dev);
+
+	return count;
+}
+static DEVICE_ATTR_WO(coredump);
+
 static int driver_sysfs_add(struct device *dev)
 {
 	int ret;
@@ -297,14 +309,26 @@ static int driver_sysfs_add(struct device *dev)
 					     BUS_NOTIFY_BIND_DRIVER, dev);
 
 	ret = sysfs_create_link(&dev->driver->p->kobj, &dev->kobj,
+				kobject_name(&dev->kobj));
+	if (ret)
+		goto fail;
+
+	ret = sysfs_create_link(&dev->kobj, &dev->driver->p->kobj,
+				"driver");
+	if (ret)
+		goto rm_dev;
+
+	if (!IS_ENABLED(CONFIG_DEV_COREDUMP) || !dev->driver->coredump ||
+	    !device_create_file(dev, &dev_attr_coredump))
+		return 0;
+
+	sysfs_remove_link(&dev->kobj, "driver");
+
+rm_dev:
+	sysfs_remove_link(&dev->driver->p->kobj,
 			  kobject_name(&dev->kobj));
-	if (ret == 0) {
-		ret = sysfs_create_link(&dev->kobj, &dev->driver->p->kobj,
-					"driver");
-		if (ret)
-			sysfs_remove_link(&dev->driver->p->kobj,
-					kobject_name(&dev->kobj));
-	}
+
+fail:
 	return ret;
 }
 
@@ -313,6 +337,8 @@ static void driver_sysfs_remove(struct device *dev)
 	struct device_driver *drv = dev->driver;
 
 	if (drv) {
+		if (drv->coredump)
+			device_remove_file(dev, &dev_attr_coredump);
 		sysfs_remove_link(&drv->p->kobj, kobject_name(&dev->kobj));
 		sysfs_remove_link(&dev->kobj, "driver");
 	}
diff --git a/include/linux/device.h b/include/linux/device.h
index 46cece5..cd3b47e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -287,6 +287,7 @@ struct device_driver {
 	const struct attribute_group **groups;
 
 	const struct dev_pm_ops *pm;
+	int (*coredump) (struct device *dev);
 
 	struct driver_private *p;
 };
@@ -300,7 +301,6 @@ extern struct device_driver *driver_find(const char *name,
 extern int driver_probe_done(void);
 extern void wait_for_device_probe(void);
 
-
 /* sysfs interface for exporting driver attributes */
 
 struct driver_attribute {
-- 
1.9.1

Re: [PATCH 0/2] sysfs: allow user-space request for devcoredump

[Greg Kroah-Hartman]

On Tue, Dec 19, 2017 at 11:03:20AM +0100, Arend van Spriel wrote:
> From: Arend van Spriel <aspriel@gmail.com>
> 
> Since commit 833c95456a70 ("device coredump: add new device coredump class")
> device drivers have a unified way to provide binary data obtained from a
> failing_device to user-space. However, there may be use-cases in which the
> driver has no reason to obtain the data, but user-space wants to initiate
> it. This adds a coredump device attribute in sysfs when the driver bound to
> the device supports the newly added coredump driver callback.

What driver is going to set this?  I don't want to add new options to
the kernel that never get used, do you have a driver to use it?

thanks,

greg k-h

Fwd: [PATCH 0/2] sysfs: allow user-space request for devcoredump

[Arend van Spriel]

+ LKML

---------- Forwarded message ----------
From: Arend van Spriel <aspriel@gmail.com>
Date: Tue, Jan 9, 2018 at 8:19 PM
Subject: Re: [PATCH 0/2] sysfs: allow user-space request for devcoredump
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


On Tue, Jan 9, 2018 at 7:46 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Dec 19, 2017 at 11:03:20AM +0100, Arend van Spriel wrote:
>> From: Arend van Spriel <aspriel@gmail.com>
>>
>> Since commit 833c95456a70 ("device coredump: add new device coredump class")
>> device drivers have a unified way to provide binary data obtained from a
>> failing_device to user-space. However, there may be use-cases in which the
>> driver has no reason to obtain the data, but user-space wants to initiate
>> it. This adds a coredump device attribute in sysfs when the driver bound to
>> the device supports the newly added coredump driver callback.
>
> What driver is going to set this?  I don't want to add new options to
> the kernel that never get used, do you have a driver to use it?

Hi Greg,

After sending these patches I realized that was going to be your
response, but decided to wait for it. Obviously there is a driver for
which I would like to have an option to create a device coredump from
user-space, ie. the brcm80211 drivers (maintained by my alter-ego ;-)
). I can post these patches again as RFC including the brcm80211
driver patches. Here a bit of background about this itch. A couple of
other driver support their own coredump implementation, ie. not using
dev_coredump class and some support user-space initiated coredump
through debugfs. As that depends on a Kconfig option I looked for
other solutions. At first I considered doing it in dev_coredump class
itself by doing:

# echo 0000:04:00.0 > /sys/class/devcoredump/trigger

with 'trigger' being a new sysfs attribute for devcoredump. However,
using the driver core turned out to be much simpler, but I am still
open for other/better options.

Regards,
Arend

Re: Fwd: [PATCH 0/2] sysfs: allow user-space request for devcoredump

[Greg KH]

On Tue, Jan 09, 2018 at 08:21:11PM +0100, Arend van Spriel wrote:
> + LKML
> 
> ---------- Forwarded message ----------
> From: Arend van Spriel <aspriel@gmail.com>
> Date: Tue, Jan 9, 2018 at 8:19 PM
> Subject: Re: [PATCH 0/2] sysfs: allow user-space request for devcoredump
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> 
> On Tue, Jan 9, 2018 at 7:46 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Dec 19, 2017 at 11:03:20AM +0100, Arend van Spriel wrote:
> >> From: Arend van Spriel <aspriel@gmail.com>
> >>
> >> Since commit 833c95456a70 ("device coredump: add new device coredump class")
> >> device drivers have a unified way to provide binary data obtained from a
> >> failing_device to user-space. However, there may be use-cases in which the
> >> driver has no reason to obtain the data, but user-space wants to initiate
> >> it. This adds a coredump device attribute in sysfs when the driver bound to
> >> the device supports the newly added coredump driver callback.
> >
> > What driver is going to set this?  I don't want to add new options to
> > the kernel that never get used, do you have a driver to use it?
> 
> Hi Greg,
> 
> After sending these patches I realized that was going to be your
> response, but decided to wait for it. Obviously there is a driver for
> which I would like to have an option to create a device coredump from
> user-space, ie. the brcm80211 drivers (maintained by my alter-ego ;-)
> ). I can post these patches again as RFC including the brcm80211
> driver patches. Here a bit of background about this itch. A couple of
> other driver support their own coredump implementation, ie. not using
> dev_coredump class and some support user-space initiated coredump
> through debugfs. As that depends on a Kconfig option I looked for
> other solutions. At first I considered doing it in dev_coredump class
> itself by doing:
> 
> # echo 0000:04:00.0 > /sys/class/devcoredump/trigger
> 
> with 'trigger' being a new sysfs attribute for devcoredump. However,
> using the driver core turned out to be much simpler, but I am still
> open for other/better options.

Nah, I don't object to the patches, but I want someone to use it.  So
can you resend these, with a RFC for the driver you want using it as the
last patch?

thanks,

greg k-h

Re: Fwd: [PATCH 0/2] sysfs: allow user-space request for devcoredump

[Arend van Spriel]

On Tue, Jan 9, 2018 at 9:03 PM, Greg KH <greg@kroah.com> wrote:
> On Tue, Jan 09, 2018 at 08:21:11PM +0100, Arend van Spriel wrote:
>> + LKML
>>
>> ---------- Forwarded message ----------
>> From: Arend van Spriel <aspriel@gmail.com>
>> Date: Tue, Jan 9, 2018 at 8:19 PM
>> Subject: Re: [PATCH 0/2] sysfs: allow user-space request for devcoredump
>> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>
>>
>> On Tue, Jan 9, 2018 at 7:46 PM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>> > On Tue, Dec 19, 2017 at 11:03:20AM +0100, Arend van Spriel wrote:
>> >> From: Arend van Spriel <aspriel@gmail.com>
>> >>
>> >> Since commit 833c95456a70 ("device coredump: add new device coredump class")
>> >> device drivers have a unified way to provide binary data obtained from a
>> >> failing_device to user-space. However, there may be use-cases in which the
>> >> driver has no reason to obtain the data, but user-space wants to initiate
>> >> it. This adds a coredump device attribute in sysfs when the driver bound to
>> >> the device supports the newly added coredump driver callback.
>> >
>> > What driver is going to set this?  I don't want to add new options to
>> > the kernel that never get used, do you have a driver to use it?
>>
>> Hi Greg,
>>
>> After sending these patches I realized that was going to be your
>> response, but decided to wait for it. Obviously there is a driver for
>> which I would like to have an option to create a device coredump from
>> user-space, ie. the brcm80211 drivers (maintained by my alter-ego ;-)
>> ). I can post these patches again as RFC including the brcm80211
>> driver patches. Here a bit of background about this itch. A couple of
>> other driver support their own coredump implementation, ie. not using
>> dev_coredump class and some support user-space initiated coredump
>> through debugfs. As that depends on a Kconfig option I looked for
>> other solutions. At first I considered doing it in dev_coredump class
>> itself by doing:
>>
>> # echo 0000:04:00.0 > /sys/class/devcoredump/trigger
>>
>> with 'trigger' being a new sysfs attribute for devcoredump. However,
>> using the driver core turned out to be much simpler, but I am still
>> open for other/better options.
>
> Nah, I don't object to the patches, but I want someone to use it.  So
> can you resend these, with a RFC for the driver you want using it as the
> last patch?

Thanks. Will do that. FWIW, there are a couple more drivers that I
would like to change to using this as well.

Regards,
Arend

Re: Fwd: [PATCH 0/2] sysfs: allow user-space request for devcoredump

[Greg KH]

On Tue, Jan 09, 2018 at 09:06:21PM +0100, Arend van Spriel wrote:
> On Tue, Jan 9, 2018 at 9:03 PM, Greg KH <greg@kroah.com> wrote:
> > On Tue, Jan 09, 2018 at 08:21:11PM +0100, Arend van Spriel wrote:
> >> + LKML
> >>
> >> ---------- Forwarded message ----------
> >> From: Arend van Spriel <aspriel@gmail.com>
> >> Date: Tue, Jan 9, 2018 at 8:19 PM
> >> Subject: Re: [PATCH 0/2] sysfs: allow user-space request for devcoredump
> >> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>
> >>
> >> On Tue, Jan 9, 2018 at 7:46 PM, Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org> wrote:
> >> > On Tue, Dec 19, 2017 at 11:03:20AM +0100, Arend van Spriel wrote:
> >> >> From: Arend van Spriel <aspriel@gmail.com>
> >> >>
> >> >> Since commit 833c95456a70 ("device coredump: add new device coredump class")
> >> >> device drivers have a unified way to provide binary data obtained from a
> >> >> failing_device to user-space. However, there may be use-cases in which the
> >> >> driver has no reason to obtain the data, but user-space wants to initiate
> >> >> it. This adds a coredump device attribute in sysfs when the driver bound to
> >> >> the device supports the newly added coredump driver callback.
> >> >
> >> > What driver is going to set this?  I don't want to add new options to
> >> > the kernel that never get used, do you have a driver to use it?
> >>
> >> Hi Greg,
> >>
> >> After sending these patches I realized that was going to be your
> >> response, but decided to wait for it. Obviously there is a driver for
> >> which I would like to have an option to create a device coredump from
> >> user-space, ie. the brcm80211 drivers (maintained by my alter-ego ;-)
> >> ). I can post these patches again as RFC including the brcm80211
> >> driver patches. Here a bit of background about this itch. A couple of
> >> other driver support their own coredump implementation, ie. not using
> >> dev_coredump class and some support user-space initiated coredump
> >> through debugfs. As that depends on a Kconfig option I looked for
> >> other solutions. At first I considered doing it in dev_coredump class
> >> itself by doing:
> >>
> >> # echo 0000:04:00.0 > /sys/class/devcoredump/trigger
> >>
> >> with 'trigger' being a new sysfs attribute for devcoredump. However,
> >> using the driver core turned out to be much simpler, but I am still
> >> open for other/better options.
> >
> > Nah, I don't object to the patches, but I want someone to use it.  So
> > can you resend these, with a RFC for the driver you want using it as the
> > last patch?
> 
> Thanks. Will do that. FWIW, there are a couple more drivers that I
> would like to change to using this as well.

Great, do it for all of them!