diff mbox series

[v4,2/5] devcoredump: Add per device sysfs entry to enable/disable coredump

Message ID 20220809083112.v4.2.Ief1110784c6c1c3ac0ee5677c2d28d785af8686d@changeid
State New
Headers show
Series [v4,1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled | expand

Commit Message

Manish Mandlik Aug. 9, 2022, 3:35 p.m. UTC 
The /sys/class/devcoredump/disabled provides only one-way disable
functionality. Also, disabling devcoredump using it disables the
devcoredump functionality for everyone who is using it.

Provide a way to selectively enable/disable devcoredump for the device
which is bound to a driver that implements the '.coredump()' callback.

This adds the 'coredump_disabled' driver attribute. When the driver
implements the '.coredump()' callback, 'coredump_disabled' file is added
along with the 'coredump' file in the sysfs folder of the device upon
driver binding. The file is removed when the driver is unbound.

Drivers can use this attribute to enable/disable devcoredump and the
userspace can write 0 or 1 to /sys/devices/.../coredump_disabled sysfs
entry to control enabling/disabling of devcoredump for that device.

Signed-off-by: Manish Mandlik <mmandlik@google.com>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

Changes in v4:
- New patch in the series

 drivers/base/dd.c          | 43 +++++++++++++++++++++++++++++++++++---
 drivers/base/devcoredump.c |  2 +-
 include/linux/device.h     |  4 ++++
 3 files changed, 45 insertions(+), 4 deletions(-)

Comments

Greg KH Aug. 9, 2022, 5:27 p.m. UTC | #1 
On Tue, Aug 09, 2022 at 08:35:24AM -0700, Manish Mandlik wrote:
> The /sys/class/devcoredump/disabled provides only one-way disable
> functionality. Also, disabling devcoredump using it disables the
> devcoredump functionality for everyone who is using it.
> 
> Provide a way to selectively enable/disable devcoredump for the device
> which is bound to a driver that implements the '.coredump()' callback.
> 
> This adds the 'coredump_disabled' driver attribute. When the driver
> implements the '.coredump()' callback, 'coredump_disabled' file is added
> along with the 'coredump' file in the sysfs folder of the device upon
> driver binding. The file is removed when the driver is unbound.
> 
> Drivers can use this attribute to enable/disable devcoredump and the
> userspace can write 0 or 1 to /sys/devices/.../coredump_disabled sysfs
> entry to control enabling/disabling of devcoredump for that device.
> 
> Signed-off-by: Manish Mandlik <mmandlik@google.com>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
> 
> Changes in v4:
> - New patch in the series
> 
>  drivers/base/dd.c          | 43 +++++++++++++++++++++++++++++++++++---
>  drivers/base/devcoredump.c |  2 +-
>  include/linux/device.h     |  4 ++++
>  3 files changed, 45 insertions(+), 4 deletions(-)

You can't add a new sysfs file without also a Documentation/ABI update
at the same time :(

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 11b0fb6414d3..c76d1145c6d9 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -426,6 +426,31 @@  static ssize_t coredump_store(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_WO(coredump);
 
+static ssize_t coredump_disabled_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	return scnprintf(buf, 3, "%d\n", dev->coredump_disabled);
+}
+
+static ssize_t coredump_disabled_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	long coredump_disabled;
+
+	if (!kstrtol(buf, 10, &coredump_disabled)) {
+		/* Consider any non-zero value as true */
+		if (coredump_disabled)
+			dev->coredump_disabled = true;
+		else
+			dev->coredump_disabled = false;
+	}
+
+	return count;
+}
+static DEVICE_ATTR_RW(coredump_disabled);
+
 static int driver_sysfs_add(struct device *dev)
 {
 	int ret;
@@ -448,9 +473,19 @@  static int driver_sysfs_add(struct device *dev)
 		return 0;
 
 	ret = device_create_file(dev, &dev_attr_coredump);
-	if (!ret)
-		return 0;
+	if (ret)
+		goto rm_driver;
+
+	ret = device_create_file(dev, &dev_attr_coredump_disabled);
+	if (ret)
+		goto rm_coredump;
 
+	return 0;
+
+rm_coredump:
+	device_remove_file(dev, &dev_attr_coredump);
+
+rm_driver:
 	sysfs_remove_link(&dev->kobj, "driver");
 
 rm_dev:
@@ -466,8 +501,10 @@  static void driver_sysfs_remove(struct device *dev)
 	struct device_driver *drv = dev->driver;
 
 	if (drv) {
-		if (drv->coredump)
+		if (drv->coredump) {
+			device_remove_file(dev, &dev_attr_coredump_disabled);
 			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/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
index f4d794d6bb85..c5e9af9f3181 100644
--- a/drivers/base/devcoredump.c
+++ b/drivers/base/devcoredump.c
@@ -255,7 +255,7 @@  void dev_coredumpm(struct device *dev, struct module *owner,
 	struct devcd_entry *devcd;
 	struct device *existing;
 
-	if (devcd_disabled)
+	if (devcd_disabled || dev->coredump_disabled)
 		goto free;
 
 	existing = class_find_device(&devcd_class, NULL, dev,
diff --git a/include/linux/device.h b/include/linux/device.h
index dc941997795c..120dd656f99d 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -469,6 +469,8 @@  struct device_physical_location {
  * 		This identifies the device type and carries type-specific
  * 		information.
  * @mutex:	Mutex to synchronize calls to its driver.
+ * @coredump_disabled: Can be used by drivers to selectively enable/disable the
+ *		coredump for a particular device via sysfs entry.
  * @bus:	Type of bus device is on.
  * @driver:	Which driver has allocated this
  * @platform_data: Platform data specific to the device.
@@ -561,6 +563,8 @@  struct device {
 	const char		*init_name; /* initial name of the device */
 	const struct device_type *type;
 
+	bool			coredump_disabled;
+
 	struct bus_type	*bus;		/* type of bus device is on */
 	struct device_driver *driver;	/* which driver has allocated this
 					   device */