2014-04-24 06:35:54

by Rusty Russell

[permalink] [raw]
Subject: [PATCH] module: remove warning about waiting module removal.

We remove the waiting module removal in commit 3f2b9c9cdf38 (September
2013), but it turns out that modprobe in kmod (< version 16) was
asking for waiting module removal. Noone noticed since modprobe would
check for 0 usage immediately before trying to remove the module, and
the race is unlikely.

However, it means that anyone running old (but not ancient) kmod
versions is hitting the printk designed to see if anyone was running
"rmmod -w". All reports so far have been false positives, so remove
the warning.

Fixes: 3f2b9c9cdf389e303b2273679af08aab5f153517
Reported-by: Valerio Vanni <[email protected]>
Cc: Elliott, Robert (Server Storage) <[email protected]>
Cc: [email protected]
Signed-off-by: Rusty Russell <[email protected]>

diff --git a/kernel/module.c b/kernel/module.c
index 11869408f79b..ae7821898bf2 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -815,9 +815,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
return -EFAULT;
name[MODULE_NAME_LEN-1] = '\0';

- if (!(flags & O_NONBLOCK))
- pr_warn("waiting module removal not supported: please upgrade\n");
-
if (mutex_lock_interruptible(&module_mutex) != 0)
return -EINTR;


2014-04-24 11:15:45

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH] module: remove warning about waiting module removal.

Hi Rusty,

On Thu, Apr 24, 2014 at 3:24 AM, Rusty Russell <[email protected]> wrote:
> We remove the waiting module removal in commit 3f2b9c9cdf38 (September
> 2013), but it turns out that modprobe in kmod (< version 16) was
> asking for waiting module removal. Noone noticed since modprobe would
> check for 0 usage immediately before trying to remove the module, and
> the race is unlikely.
>
> However, it means that anyone running old (but not ancient) kmod
> versions is hitting the printk designed to see if anyone was running
> "rmmod -w". All reports so far have been false positives, so remove
> the warning.
>
> Fixes: 3f2b9c9cdf389e303b2273679af08aab5f153517
> Reported-by: Valerio Vanni <[email protected]>
> Cc: Elliott, Robert (Server Storage) <[email protected]>
> Cc: [email protected]
> Signed-off-by: Rusty Russell <[email protected]>
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 11869408f79b..ae7821898bf2 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -815,9 +815,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
> return -EFAULT;
> name[MODULE_NAME_LEN-1] = '\0';
>
> - if (!(flags & O_NONBLOCK))
> - pr_warn("waiting module removal not supported: please upgrade\n");
> -
> if (mutex_lock_interruptible(&module_mutex) != 0)
> return -EINTR;
>

Ack.

If you are going to apply this, do you think it'd be still good to
patch kmod on distros, so at least modprobe -r uses O_NONBLOCK? Or
having this patch in kernel is sufficient?

--
Lucas De Marchi

2014-04-28 02:24:54

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] module: remove warning about waiting module removal.

Lucas De Marchi <[email protected]> writes:
> Hi Rusty,
>
> On Thu, Apr 24, 2014 at 3:24 AM, Rusty Russell <[email protected]> wrote:
>> We remove the waiting module removal in commit 3f2b9c9cdf38 (September
>> 2013), but it turns out that modprobe in kmod (< version 16) was
>> asking for waiting module removal. Noone noticed since modprobe would
>> check for 0 usage immediately before trying to remove the module, and
>> the race is unlikely.
>>
>> However, it means that anyone running old (but not ancient) kmod
>> versions is hitting the printk designed to see if anyone was running
>> "rmmod -w". All reports so far have been false positives, so remove
>> the warning.
>>
>> Fixes: 3f2b9c9cdf389e303b2273679af08aab5f153517
>> Reported-by: Valerio Vanni <[email protected]>
>> Cc: Elliott, Robert (Server Storage) <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: Rusty Russell <[email protected]>
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 11869408f79b..ae7821898bf2 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -815,9 +815,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>> return -EFAULT;
>> name[MODULE_NAME_LEN-1] = '\0';
>>
>> - if (!(flags & O_NONBLOCK))
>> - pr_warn("waiting module removal not supported: please upgrade\n");
>> -
>> if (mutex_lock_interruptible(&module_mutex) != 0)
>> return -EINTR;
>>
>
> Ack.
>
> If you are going to apply this, do you think it'd be still good to
> patch kmod on distros, so at least modprobe -r uses O_NONBLOCK? Or
> having this patch in kernel is sufficient?

I think it's sufficient: distros will get this patch via stable. kmod
updates can occur at a natural pace.

Thanks,
Rusty.