Re: [PATCH v5 0/2] Small devm helper for devm implementations

[Greg Kroah-Hartman]

On Mon, May 03, 2021 at 02:08:33PM +0200, Marc Gonzalez wrote:
> On 23/07/2020 17:00, Greg Kroah-Hartman wrote:
> 
> > On Tue, Jul 21, 2020 at 09:10:23AM +0200, Marc Gonzalez wrote:
> > 
> >> On 06/07/2020 21:57, Greg Kroah-Hartman wrote:
> >>
> >>> Given the lack of testing of the patch, it doesn't seem wise to add
> >>> this, right?
> >>
> >> You're probably not wrong :)
> >>
> >>> Please get some testing, and some more users, and I'll be glad to
> >>> consider it.
> >>
> >> "Users" == files modified to use the new helper?
> > 
> > Yes.
> > 
> >> How many files would you suggest? 3? 5? 10?
> > 
> > How many do you see that can use it?  I would suggest "all" :)
> 
> Hello Greg (and everyone on the CC list),
> 
> I'm getting the itch to work on this patch-set again.
> 
> To recap: I wrote a tiny devm helper.
> It's a trivial wrapper around devres_alloc() + devres_add()
> which releases the resource when devres_alloc() fails.
> That's all there is to it.
> 
> Despite its triviality, the helper allows writing simpler code
> in drivers using devm, and generates smaller object code,
> so I think it's quite useful.
> 
> 
> With all that being said, I'm a bit concerned by Greg's "all" answer.
> 
> $ git grep '= devres_alloc' | wc -l
> 173
> 
> $ git grep -c '= devres_alloc' | wc -l
> 103
> 
> There are 173 calls to devres_alloc across 103 files.
> 
> It looks (to me) too risky to change everything in a single patch-set.
> 
> Perhaps we could define a few frameworks that would get the improvement
> as a first step?
> 
> $ git grep '= devres_alloc' v5.12 | grep -o 'v5.12:[[:alnum:]]*/[[:alnum:]]*' | uniq -c
>       1 v5.12:Documentation/driver
>       3 v5.12:drivers/ata
>      11 v5.12:drivers/base
>       1 v5.12:drivers/bus
>       1 v5.12:drivers/char
>      13 v5.12:drivers/clk
>       1 v5.12:drivers/counter
>       4 v5.12:drivers/devfreq
>       2 v5.12:drivers/dma
>       4 v5.12:drivers/extcon
>       2 v5.12:drivers/firmware
>       4 v5.12:drivers/fpga
>       8 v5.12:drivers/gpio
>       1 v5.12:drivers/gpu
>       2 v5.12:drivers/hid
>       2 v5.12:drivers/hwmon
>       3 v5.12:drivers/hwspinlock
>       1 v5.12:drivers/i2c
>      12 v5.12:drivers/iio
>       2 v5.12:drivers/input
>       1 v5.12:drivers/interconnect
>       5 v5.12:drivers/leds
>       1 v5.12:drivers/macintosh
>       1 v5.12:drivers/mailbox
>       2 v5.12:drivers/media
>       2 v5.12:drivers/mfd
>       1 v5.12:drivers/mtd
>       3 v5.12:drivers/mux
>       6 v5.12:drivers/net
>       1 v5.12:drivers/ntb
>       3 v5.12:drivers/nvmem
>       1 v5.12:drivers/of
>       4 v5.12:drivers/pci
>       5 v5.12:drivers/phy
>       3 v5.12:drivers/pinctrl
>       2 v5.12:drivers/platform
>       4 v5.12:drivers/power
>       3 v5.12:drivers/pwm
>       6 v5.12:drivers/regulator
>       1 v5.12:drivers/remoteproc
>       3 v5.12:drivers/reset
>       3 v5.12:drivers/spi
>       2 v5.12:drivers/staging
>       3 v5.12:drivers/thermal
>       1 v5.12:drivers/tty
>       1 v5.12:drivers/uio
>       2 v5.12:drivers/usb
>       2 v5.12:drivers/video
>       1 v5.12:drivers/watchdog
>       1 v5.12:kernel/dma
>       1 v5.12:kernel/iomem
>       5 v5.12:kernel/irq
>       1 v5.12:kernel/reboot
>       2 v5.12:kernel/resource
>       3 v5.12:lib/devres
>       1 v5.12:lib/genalloc
>       1 v5.12:mm/dmapool
>       2 v5.12:net/devres
>       1 v5.12:sound/hda
>       6 v5.12:sound/soc
>       1 v5.12:tools/testing
> 
> 
> Perhaps I could take a look at all the subsystems/frameworks that you maintain, Greg?
> What do you think?

Just do a few, not all to start with by any means.  Just examples of how
this will be used to show if it really does make things better or not.

And sure, do USB and TTY to start with if that makes it easier.

thanks,

greg k-h