linux-kernel - Re: [RFC PATCH 0/7] Add managed version of delayed work init

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 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 19 Feb 2021 12:35:51 +0200
From:   Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
To:     mgross@...ux.intel.com
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Hans de Goede <hdegoede@...hat.com>,
        Sebastian Reichel <sre@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Saravana Kannan <saravanak@...gle.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Joerg Roedel <jroedel@...e.de>,
        Dan Williams <dan.j.williams@...el.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-hwmon@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        linux-pm@...r.kernel.org, linux-watchdog@...r.kernel.org
Subject: Re: [RFC PATCH 0/7] Add managed version of delayed work init

Hello Mark,

Thanks for taking a look at the series! This is the first time anyone
has been commenting on a cover-letter which is likely to fade away and
never be looked at again. Guess you are a thorough person :)

On Thu, 2021-02-18 at 08:28 -0800, mark gross wrote:
> On Sat, Feb 13, 2021 at 01:58:17PM +0200, Matti Vaittinen wrote:
> > It's not rare that device drivers need delayed work.
> > It's not rare that this work needs driver's data.
> > 
> > Often this means that driver must ensure the work is not queued
> > when
> > driver exits. Usually this is done by ensuring new work is not
> > added and
> > then calling cancel_delayed_work_sync() at remove(). In many cases
> > this
> > may also require cleanup at probe error path - which is easy to
> > forget.
> > 
> > It might be helpful for (a) few drivers if there was a work init
>  why the (a) and not just a?

I am not sure how many drivers are needed to change it from 'few' to 'a
few'. Additionally, this series converted only the drivers which I
found could easily get rid of the .remove() - I did not analyze how
many drivers would benefit from this by getting rid of mixed
devm/manual resource management.

So to sum up - I don't know how many drivers will benefit and what
people think makes 'few' to turn to 'a few'. '(a) few' leaves this
decision to readers - and (a) few of them know the drivers better than
I do.

> > Main reson why this is RFC is that I had hard time deciding where
> > this
> > function should be introduced. It's not nice to include all device
> > stuff
> > in workqueue - because many workqueue users are not interested in
> > devices. In same way, not all of the devices are interested in WQs.
> > OTOH, adding own file just for this sounds like an overkill.
> s/own/one

Hm. The 'own file for XXX' does not make sense for native English
speakers? Didn't now that. Thanks for pointing it out.

I will edit the cover letter when I respin this rebased on v5.12-rc1 -
and it is likely the series v2 will add this function inlined in a new
header dedicated for devm-helpers (as was suggested by Hans de Goede).

Best Regards
--Matti