From: Luis Chamberlain
Sent: 22 July 2021 23:19
There is quite a bit of tribal knowledge around proper use of try_module_get() and that it must be used only in a context which can ensure the module won't be gone during the operation. Document this little bit of tribal knowledge.
...
Some typos.
+/**
- try_module_get - yields to module removal and bumps reference count otherwise
- @module: the module we should check for
- This can be used to check if userspace has requested to remove a module,
a module be removed
- and if so let the caller give up. Otherwise it takes a reference count to
- ensure a request from userspace to remove the module cannot happen.
- Care must be taken to ensure the module cannot be removed during
- try_module_get(). This can be done by having another entity other than the
- module itself increment the module reference count, or through some other
- means which gaurantees the module could not be removed during an operation.
guarantees
- An example of this later case is using this call in a sysfs file which the
- module created. The sysfs store / read file operation is ensured to exist
^^^^^^^^^^^^^^^^^^^ Not sure what that is supposed to mean.
- and still be present by kernfs's active reference. If a sysfs file operation
- is being run, the module which created it must still exist as the module is
- in charge of removal of the sysfs file.
- The real value to try_module_get() is the module_is_live() check which
- ensures this the caller of try_module_get() can yields to userspace module
- removal requests and fail whatever it was about to process.
- */
But is the comment even right? I think you need to consider when try_module_get() can actually fail. I believe the following is right. The caller has to have valid module reference and module unload must actually be in progress - ie the ref count is zero and there are no active IO operations.
The module's unload function must (eventually) invalidate the caller's module reference to stop try_module_get() being called with a (very) stale pointer.
So there is a potentially horrid race: The module unload is going to do: driver_data->module_ref = 0; and elsewhere there'll be: ref = driver_data->module_ref; if (!ref || !try_module_get(ref)) return -error;
You have to have try_module_get() to allow the module unload function to sleep. But the above code still needs a driver lock to ensure the unload code doesn't race with the try_module_get() and the 'ref' be invalidated before try_module_get() looks at it. (eg if an interrupt defers processing.)
So there can be no 'yielding'.
I'm pretty much certain try_module_get(THIS_MODULE) is pretty much never going to fail. (It is mostly needed to give a worker thread a reference.)
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)