On Tue, Sep 21, 2021 at 1:24 AM David Laight <David.Laight(a)aculab.com> wrote:
From: Luis Chamberlain
> Sent: 17 September 2021 20:47
> When sysfs attributes use a lock also used on module removal we can
> race to deadlock. This happens when for instance a sysfs file on
> a driver is used, then at the same time we have module removal call
> trigger. The module removal call code holds a lock, and then the sysfs
> file entry waits for the same lock. While holding the lock the module
> removal tries to remove the sysfs entries, but these cannot be removed
> yet as one is waiting for a lock. This won't complete as the lock is
> already held. Likewise module removal cannot complete, and so we deadlock.
Isn't the real problem the race between a sysfs file action and the
removal of the sysfs node?
Nope, that is taken care of by kernfs.
This isn't really related to module unload - except that may
well remove some sysfs nodes.
Nope, the issue is a deadlock that can happen due to a shared lock on
module removal and a driver sysfs operation.
This is the same problem as removing any other kind of driver
There are three basic solutions:
1) Use a global lock - not usually useful.
2) Have the remove call sleep until any callbacks are complete.
3) Have the remove just request removal and have a final
callback (from a different context).
Kernfs already does a sort of combination of 1) and 2) but 1) is using
atomic reference counts.
If the remove can sleep (as in 2) then there is a requirement
on the driver code to not hold any locks across the 'remove'
that can be acquired during the callbacks.
And this is the part that kernfs has no control over since the removal
and sysfs operation are implementation specific.
Now, for sysfs, you probably only want to sleep the remove code
while a read/write is in progress - not just because the node
That probably requires marking an open node 'invalid' and
deferring delete to close.
This is already done by kernfs.
None of this requires a reference count on the module.
You are missing the point to the other aspect of the try_module_get(),
it lets you also check if module exit has been entered. By using
try_module_get() you let the module exit trump proceeding with an
operation, therefore also preventing any potential use of a shared
lock on module exit and the driver specific sysfs operation.