Re: [ANNOUNCE] kmod 30

From: Lucas De Marchi
Date: Thu Jun 30 2022 - 18:33:53 EST


On Thu, Jun 30, 2022 at 03:09:32PM -0700, Luis Chamberlain wrote:
On Thu, Jun 30, 2022 at 08:36:21AM -0700, Lucas De Marchi wrote:
- modprobe learned a --wait <MSEC> option to be used together with -r
when removing a module. This allows modprobe to keep trying the
removal if it fails because the module is still in use. An exponential backoff
time is used for further retries.

The wait behavior provided by the kernel when not passing O_NONBLOCK
to delete_module() was removed in v3.13 due to not be used and the
consequences of having to support it in the kernel. However there may
be some users, particularly on testsuites for individual susbsystems, that
would want that. So provide a userspace implementation inside modprobe for
such users. "rmmod" doesn't have a --wait as it remains a bare minimal over
the API provided by the kernel. In future the --wait behavior can be added
to libkmod for testsuites not exec'ing modprobe for module removal.

Sorry for the super late review, I was swamped. OK so the only issue
I can think of is that rmmod *used* to support the kernel wait support
with $(rmmod --wait) so wouldn't this be odd?

any reason not to use modprobe -r? Argument for rmmod supporting it in
the past is that the wait was implemented on the kernel side and rmmod
is the minimum wrapper around what the kernel provides.

On the other side, user shouldn't need to know where that is
implemented.

Over time libkmod grew much more to support loading/querying modules
rather than removing. I think for next version I will move some of the
module-removal support to libkmod rather than modprobe/rmmod. Then we
can think again on supporting that flag there.


It is why I had gone with:

-p | --remove-patiently patiently removes the module
-t | --timeout timeout in ms to remove the module

You would know better though.

Also just curious, is it really terrible to just support waiting
forever?

is there a use case for that? If we are trying to cover some races, I
imagine a small timeout would be sufficient. Also notice that if the
timeout is too big, so will be the interval between the retries. On
your v2 I had suggested polling the refcnt so we would get notificed
on changes, but as you also noticed, that didn't work very well. So I
went back to a time-based retry solution.

if there is a use-case, should we cap the interval between retries?

thanks
Lucas De Marchi


Luis