Thread (15 messages) 15 messages, 5 authors, 2018-10-18

[PATCH 2/3] driver core: add deferring probe reason to devices_deferred property

From: Andy Shevchenko <hidden>
Date: 2018-10-16 13:42:57
Also in: lkml

On Tue, Oct 16, 2018 at 10:22 AM Andrzej Hajda [off-list ref] wrote:
/sys/kernel/debug/devices_deferred property contains list of deferred devices.
This list does not contain reason why the driver deferred probe, the patch
improves it.
The natural place to set the reason is probe_err function introduced recently,
ie. if probe_err will be called with -EPROBE_DEFER instead of printk the message
will be attached to deferred device and printed when user read devices_deferred
property.
-       if (err != -EPROBE_DEFER) {
-               va_start(args, fmt);
+       va_start(args, fmt);

-               vaf.fmt = fmt;
-               vaf.va = &args;
+       vaf.fmt = fmt;
+       vaf.va = &args;

+       if (err != -EPROBE_DEFER)
                __dev_printk(KERN_ERR, dev, &vaf);
-               va_end(args);
-       }
+       else
+               __deferred_probe_set_msg(dev, fmt, args);
+
+       va_end(args);
Yeah, ping-pong style of programming (one patch "amends" something
which had been introduced by another).
That's why better to follow my comment for the patch 1.
+/*
+ * __deferred_probe_set_msg() - Set defer probe reason message for device
+ */
+void __deferred_probe_set_msg(const struct device *dev, const char *fmt,
+                                    va_list args)
+{
+       const int size = 128;
+       char **p;
+       int n;
+
+       mutex_lock(&deferred_probe_mutex);
+
+       p = &dev->p->deferred_probe_msg;
+       if (!*p) {
+               *p = kmalloc(size, GFP_KERNEL);
+               if (!*p)
+                       goto end;
+       }
+       n = snprintf(*p, size, "%s %s: ", dev_driver_string(dev), dev_name(dev));
+       if (n < size)
+               vsnprintf(*p + n, size - n, fmt, args);
Perhaps kasprintf() instead of this huge plays around the size and allocation?
        list_for_each_entry(curr, &deferred_probe_pending_list, deferred_probe)
-               seq_printf(s, "%s\n", dev_name(curr->device));
+               if (curr->device->p->deferred_probe_msg)
+                       seq_puts(s, curr->device->p->deferred_probe_msg);
+               else
+                       seq_printf(s, "%s\n", dev_name(curr->device));
I wouldn't care (much) about debugfs, though better to keep some order
here, i.e. always print device name.

Something like

seq_printf(s, "%s\t%s\n", dev_name(), deferred_probe_msg ?: "");

and remove that part from above __deferred_probe_set_msg().

-- 
With Best Regards,
Andy Shevchenko
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help