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

From: Matti Vaittinen
Date: Fri Feb 19 2021 - 05:36:54 EST


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