lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 30 Jun 2020 11:00:21 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Andrzej Hajda <a.hajda@...sung.com>
Cc:     Grygorii Strashko <grygorii.strashko@...com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Jonas Karlman <jonas@...boo.se>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        "open list:DRM DRIVERS" <dri-devel@...ts.freedesktop.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Neil Armstrong <narmstrong@...libre.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH v6 2/4] driver core: add deferring probe reason to
 devices_deferred property

On Tue, Jun 30, 2020 at 8:42 AM Andrzej Hajda <a.hajda@...sung.com> wrote:
>
>
> On 30.06.2020 10:59, Grygorii Strashko wrote:
> > Hi
> >
> > On 29/06/2020 14:28, Andrzej Hajda wrote:
> >> Hi Grygorii,
> >>
> >> (...)
> >>
> >>>>    /*
> >>>>     * deferred_devs_show() - Show the devices in the deferred probe
> >>>> pending list.
> >>>>     */
> >>>> @@ -221,7 +241,8 @@ static int deferred_devs_show(struct seq_file *s,
> >>>> void *data)
> >>>>        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));
> >>>> +        seq_printf(s, "%s\t%s", dev_name(curr->device),
> >>>> + curr->device->p->deferred_probe_reason ?: "\n");
> >>>>          mutex_unlock(&deferred_probe_mutex);
> >>>>
> >>>
> >>> Sry, may be i missing smth, but shouldn't it be optional
> >>> (CONFIG_DEBUG_FS is probably too generic).
> >>>
> >>
> >> I am not sure what exactly are you referring to, but this patch does not
> >> add new property, it just extends functionality of existing one.
> >
> > Sry, needed to be more specific.
> >
> > You've added  device_set_deferred_probe_reson(dev, &vaf);
> > which expected to be used on every EPROBE_DEFER in dev_err_probe() in
> > combination with
> >
> > +       } else {
> > +               device_set_deferred_probe_reson(dev, &vaf);
> >                 dev_dbg(dev, "error %d: %pV", err, &vaf);
> >
> > ^^ dev_dbg() does not add any runtime overhead during boot unless enabled
> > +       }
> >
> > But:
> >
> > +void device_set_deferred_probe_reson(const struct device *dev, struct
> > va_format *vaf)
> > +{
> > +       const char *drv = dev_driver_string(dev);
> > +
> > +       mutex_lock(&deferred_probe_mutex);
> > +
> > +       kfree(dev->p->deferred_probe_reason);
> > +       dev->p->deferred_probe_reason = kasprintf(GFP_KERNEL, "%s:
> > %pV", drv, vaf);
> > +
> > +       mutex_unlock(&deferred_probe_mutex);
> > +}
> >
> > ^^ Adds locking, kfree() and kasprintf() for every deferred probe
> > during boot and can't be disabled.
> >
> > Right?
>
>
> Right, but usually the burden should be insignificant in comparison to
> probe time, so I do not think it is worth optimizing.

I do not think this is going to take. You are suggesting that we
modify pretty much every driver to supply this deferral reason, and I
doubt it will happen. Can we put this burden on providers that raise
the deferral? I.e. majority of code are using devm API now, so we most
likely know the device for which deferral is being raised. We can have
a list of deferral reasons and their devices and when in device code
once probe is done we could try reconciling it with the deferred
devicelist, and this would mean you only need to implement this in
gpiolib, regulator core, clocks core, etc.

Thanks.

-- 
Dmitry

Powered by blists - more mailing lists