| Message ID | 20180619183356.32106-1-javierm@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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 >
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. >> + }
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
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,
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 */
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@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(+)