Thread (21 messages) 21 messages, 6 authors, 2022-08-30

Re: (subset) [PATCH v2 0/7] Devm helpers for regulator get and enable

From: Vaittinen, Matti <hidden>
Date: 2022-08-16 11:39:43
Also in: dri-devel, linux-amlogic, linux-clk, linux-doc, linux-hwmon, linux-iio, lkml

On 8/16/22 13:36, Mark Brown wrote:
On Tue, Aug 16, 2022 at 07:56:06AM +0300, Matti Vaittinen wrote:
quoted
On 8/16/22 01:55, Mark Brown wrote:
quoted
On Tue, Aug 16, 2022 at 12:17:17AM +0300, Laurent Pinchart wrote:
quoted
quoted
quoted
These devres helpers give
a false sense of security to driver authors and they will end up
introducing problems, the same way that devm_kzalloc() makes it
outrageously easy to crash the kernel by disconnecting a device that is
in use.
quoted
I think this is going a bit "off-topic" but I'd like to understand what is
behind this statement? From device-writer's perspective - I don't know much
better alternatives to free up the memory. I don't see how freeing stuff at
.remove would be any better? As far as I understand - if someone is using
driver's resources after the device has gone and the driver is detached,
then there is not much the driver could do to free-up the stuff be it devm
or not? This sounds like fundamentally different problem (to me).
If a driver has done something like create a file that's accessible to
userspace then that file may be held open by userspace even after the
device goes away, the driver that created the file needs to ensure that
any storage that's used by the file remains allocated until the file is
also freed (typically this is data specific to the file rather than the
full device data).  Similar situations can exist elsewhere, that's just
the common case.  There'll be a deletion callback on whatever other
object causes the problem, the allocation needs to be reference counted
against both the the device and whatever other users there might be.
Oh right. Thanks. So we're discussing (a corner?) case where the freeing 
is done by a callback that was registered by a driver. Callback being 
called for example when the refcount for a resource gets down, 
potentially long after the driver was gone.

I see the problem of releasing something with devm in such case. Still, 
I don't think it is something we should avoid by banning the use of devm 
- which is useful in many other cases. It'd be equally wrong release the 
resource in .remove() or doing any other "double freeing". To me this 
boils down to educating people about the life-times.

I wonder if writing such 'release callbacks' is compulsory? I mean, if I 
was writing a driver to some new (to me) subsystem and was required to 
write an explicit release-callback for a resource - then it'd surely 
rang a bell about potentially double freeing stuff with devm. Especially 
if the doc stated the callback can be called after the driver has been 
detached.
quoted
quoted
That's a different conversation, and a totally
valid one especially when you start looking at things like implementing
userspace APIs which need to cope with hardware going away while still
visible to userspace.
quoted
This is interesting. It's not easy for me to spot how devm changes things
here? If we consider some removable device - then I guess also the .remove()
is ran only after HW has already gone? Yes, devm might make the time window
when userspace can see hardware that has gone longer but does it bring any
new problem there? It seems to me devm can make hitting the spot more likely
- but I don't think it brings completely new issues? (Well, I may be wrong
here - wouldn't be the first time :])
See above, something can still know the device was there even after it's
gone.
Yes. Thanks for the education.
I'm still not sure I understand how devm changes the picture. I'd guess 
in the case you described above, the user-space would still see the 
device until it closes the file and the call-back cleans-up. I guess 
freeing the stuff (that is used until user-space drops the handle) 
anywhere except the clean-up call-back is wrong - be the mechanism devm 
or driver's .remove() or any other. To me the key is still teaching 
people to know what bits may be used after the driver has been detached.

Thanks for the explanation guys - this has been insightful to me :)

Best Regards
-- Matti

-- 
The Linux Kernel guy at ROHM Semiconductors

Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~ this year is the year of a signature writers block ~~
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help