[RFC PATCH v2] driver core: add a debugfs entry to show deferred devices

[RFC PATCH v2] driver core: add a debugfs entry to show deferred devices

[Javier Martinez Canillas]

For debugging purposes it may be useful to know what are the devices whose
probe function was deferred. Add a debugfs entry showing that information.

  $ cat /sys/kernel/debug/deferred_devices
  48070000.i2c:twl at 48:bci
  musb-hdrc.0.auto
  omapdrm.0

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

---

Changes in v2:
- Remove unneeded ret variable from deferred_devs_show()

 drivers/base/dd.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 1435d7281c6..d95bd4636fc 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -224,6 +224,42 @@ void device_unblock_probing(void)
 	driver_deferred_probe_trigger();
 }
 
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+#include <linux/debugfs.h>
+
+static struct dentry *deferred_devices;
+
+/*
+ * deferred_devs_show() - Show the devices in the deferred probe pending list.
+ */
+static int deferred_devs_show(struct seq_file *s, void *data)
+{
+	struct device_private *curr;
+
+	mutex_lock(&deferred_probe_mutex);
+
+	list_for_each_entry(curr, &deferred_probe_pending_list, deferred_probe)
+		seq_printf(s, "%s\n", dev_name(curr->device));
+
+	mutex_unlock(&deferred_probe_mutex);
+
+	return 0;
+}
+
+static int deferred_devs_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, deferred_devs_show, inode->i_private);
+}
+
+static const struct file_operations deferred_devs_fops = {
+	.owner = THIS_MODULE,
+	.open = deferred_devs_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+#endif /* IS_ENABLED(CONFIG_DEBUG_FS) */
+
 /**
  * deferred_probe_initcall() - Enable probing of deferred devices
  *
@@ -233,6 +269,14 @@ void device_unblock_probing(void)
  */
 static int deferred_probe_initcall(void)
 {
+	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
+		deferred_devices = debugfs_create_file("deferred_devices",
+						       0444, NULL, NULL,
+						       &deferred_devs_fops);
+		if (!deferred_devices)
+			return -ENOMEM;
+	}
+
 	driver_deferred_probe_enable = true;
 	driver_deferred_probe_trigger();
 	/* Sort as many dependencies as possible before exiting initcalls */
-- 
2.17.1

[RFC PATCH v2] driver core: add a debugfs entry to show deferred devices

[Andy Shevchenko]

On Tue, Jun 19, 2018 at 9:33 PM, Javier Martinez Canillas
<javierm@redhat.com> wrote:
> For debugging purposes it may be useful to know what are the devices whose
> probe function was deferred. Add a debugfs entry showing that information.
>
>   $ cat /sys/kernel/debug/deferred_devices
>   48070000.i2c:twl at 48:bci
>   musb-hdrc.0.auto
>   omapdrm.0

> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> +#include <linux/debugfs.h>
> +
> +static struct dentry *deferred_devices;
> +
> +/*
> + * deferred_devs_show() - Show the devices in the deferred probe pending list.
> + */
> +static int deferred_devs_show(struct seq_file *s, void *data)
> +{
> +       struct device_private *curr;
> +
> +       mutex_lock(&deferred_probe_mutex);
> +
> +       list_for_each_entry(curr, &deferred_probe_pending_list, deferred_probe)
> +               seq_printf(s, "%s\n", dev_name(curr->device));
> +
> +       mutex_unlock(&deferred_probe_mutex);
> +
> +       return 0;
> +}
> +

> +static int deferred_devs_open(struct inode *inode, struct file *file)
> +{
> +       return single_open(file, deferred_devs_show, inode->i_private);
> +}
> +
> +static const struct file_operations deferred_devs_fops = {
> +       .owner = THIS_MODULE,
> +       .open = deferred_devs_open,
> +       .read = seq_read,
> +       .llseek = seq_lseek,
> +       .release = single_release,
> +};

Isn't this DEFINE_SHOW_ATTRIBUTE() ?

> +#endif /* IS_ENABLED(CONFIG_DEBUG_FS) */
> +
>  /**
>   * deferred_probe_initcall() - Enable probing of deferred devices
>   *
> @@ -233,6 +269,14 @@ void device_unblock_probing(void)
>   */
>  static int deferred_probe_initcall(void)
>  {
> +       if (IS_ENABLED(CONFIG_DEBUG_FS)) {
> +               deferred_devices = debugfs_create_file("deferred_devices",
> +                                                      0444, NULL, NULL,
> +                                                      &deferred_devs_fops);
> +               if (!deferred_devices)
> +                       return -ENOMEM;
> +       }
> +
>         driver_deferred_probe_enable = true;
>         driver_deferred_probe_trigger();
>         /* Sort as many dependencies as possible before exiting initcalls */
> --
> 2.17.1
>



-- 
With Best Regards,
Andy Shevchenko

[RFC PATCH v2] driver core: add a debugfs entry to show deferred devices

[Andy Shevchenko]

On Tue, Jun 19, 2018 at 10:53 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Jun 19, 2018 at 9:33 PM, Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> For debugging purposes it may be useful to know what are the devices whose
>> probe function was deferred. Add a debugfs entry showing that information.

>> +static int deferred_devs_open(struct inode *inode, struct file *file)
>> +{
>> +       return single_open(file, deferred_devs_show, inode->i_private);
>> +}
>> +
>> +static const struct file_operations deferred_devs_fops = {
>> +       .owner = THIS_MODULE,
>> +       .open = deferred_devs_open,
>> +       .read = seq_read,
>> +       .llseek = seq_lseek,
>> +       .release = single_release,
>> +};
>
> Isn't this DEFINE_SHOW_ATTRIBUTE() ?

Besides that, you are summoning Greg's dark side :-)
See below.

>> +       if (IS_ENABLED(CONFIG_DEBUG_FS)) {
>> +               deferred_devices = debugfs_create_file("deferred_devices",
>> +                                                      0444, NULL, NULL,
>> +                                                      &deferred_devs_fops);

>> +               if (!deferred_devices)
>> +                       return -ENOMEM;

This must not prevent the execution. So, the check introduces actually
a regression.

>> +       }

-- 
With Best Regards,
Andy Shevchenko

[RFC PATCH v2] driver core: add a debugfs entry to show deferred devices

[Greg Kroah-Hartman]

On Tue, Jun 19, 2018 at 10:55:20PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 19, 2018 at 10:53 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Tue, Jun 19, 2018 at 9:33 PM, Javier Martinez Canillas
> > <javierm@redhat.com> wrote:
> >> For debugging purposes it may be useful to know what are the devices whose
> >> probe function was deferred. Add a debugfs entry showing that information.
> 
> >> +static int deferred_devs_open(struct inode *inode, struct file *file)
> >> +{
> >> +       return single_open(file, deferred_devs_show, inode->i_private);
> >> +}
> >> +
> >> +static const struct file_operations deferred_devs_fops = {
> >> +       .owner = THIS_MODULE,
> >> +       .open = deferred_devs_open,
> >> +       .read = seq_read,
> >> +       .llseek = seq_lseek,
> >> +       .release = single_release,
> >> +};
> >
> > Isn't this DEFINE_SHOW_ATTRIBUTE() ?
> 
> Besides that, you are summoning Greg's dark side :-)
> See below.
> 
> >> +       if (IS_ENABLED(CONFIG_DEBUG_FS)) {
> >> +               deferred_devices = debugfs_create_file("deferred_devices",
> >> +                                                      0444, NULL, NULL,
> >> +                                                      &deferred_devs_fops);
> 
> >> +               if (!deferred_devices)
> >> +                       return -ENOMEM;
> 
> This must not prevent the execution. So, the check introduces actually
> a regression.

Awe, you beat me to it :)

Also, I don't usually comment on RFC patches, as that shows the author
really doesn't think that the code is ready to be reviewed/merged...

thanks,

greg k-h

[RFC PATCH v2] driver core: add a debugfs entry to show deferred devices

[Javier Martinez Canillas]

Hello Andy,

Thanks a lot for your feedback.

On 06/19/2018 09:55 PM, Andy Shevchenko wrote:
> On Tue, Jun 19, 2018 at 10:53 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Tue, Jun 19, 2018 at 9:33 PM, Javier Martinez Canillas
>> <javierm@redhat.com> wrote:
>>> For debugging purposes it may be useful to know what are the devices whose
>>> probe function was deferred. Add a debugfs entry showing that information.
> 
>>> +static int deferred_devs_open(struct inode *inode, struct file *file)
>>> +{
>>> +       return single_open(file, deferred_devs_show, inode->i_private);
>>> +}
>>> +
>>> +static const struct file_operations deferred_devs_fops = {
>>> +       .owner = THIS_MODULE,
>>> +       .open = deferred_devs_open,
>>> +       .read = seq_read,
>>> +       .llseek = seq_lseek,
>>> +       .release = single_release,
>>> +};
>>
>> Isn't this DEFINE_SHOW_ATTRIBUTE() ?
>

Indeed.
 
> Besides that, you are summoning Greg's dark side :-)
> See below.
> 
>>> +       if (IS_ENABLED(CONFIG_DEBUG_FS)) {
>>> +               deferred_devices = debugfs_create_file("deferred_devices",
>>> +                                                      0444, NULL, NULL,
>>> +                                                      &deferred_devs_fops);
> 
>>> +               if (!deferred_devices)
>>> +                       return -ENOMEM;
> 
> This must not prevent the execution. So, the check introduces actually
> a regression.
>

Fair enough, I'll fix these an re-spin the patch.
 
>>> +       }
> 

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat