Re: [RFC 2/3] module: Ignore delete_id parameter

[Jon Derrick]

On 09/28/2017 12:03 AM, Dan Williams wrote:
> On Wed, Sep 27, 2017 at 1:40 PM, Jon Derrick  
> wrote:
>> The PCI driver delete_id parameter is handled in each individual driver
>> registration callback.
>>
>> Signed-off-by: Jon Derrick 
>> ---
>>  kernel/module.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index de66ec8..2b2dccf 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
>> *val, const char *modname,
>> return 0;
>> }
>>
>> +   /*
>> +* Ignore driver delete list arguments. They are handled by driver
>> +* registration callbacks
>> +*/
>> +   if (strcmp(param, "delete_id") == 0)
>> +   return 0;
>> +
> 
> Does this preclude something like:
> 
> modprobe ahci delete_id=1234:5678?
> 

It does seem like it would. I can look into calling into the pci
callback for this, but val is a struct module here and I haven't figured
out the plumbing to get the [correct] driver from that.

Maybe if I enforce the format of 'modprobe ahci ahci.delete_id=' to
ensure the driver is specified (and would be required in cases with
multi-driver modules).

Re: [RFC 2/3] module: Ignore delete_id parameter

[Jon Derrick]

On 09/28/2017 12:03 AM, Dan Williams wrote:
> On Wed, Sep 27, 2017 at 1:40 PM, Jon Derrick  
> wrote:
>> The PCI driver delete_id parameter is handled in each individual driver
>> registration callback.
>>
>> Signed-off-by: Jon Derrick 
>> ---
>>  kernel/module.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index de66ec8..2b2dccf 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
>> *val, const char *modname,
>> return 0;
>> }
>>
>> +   /*
>> +* Ignore driver delete list arguments. They are handled by driver
>> +* registration callbacks
>> +*/
>> +   if (strcmp(param, "delete_id") == 0)
>> +   return 0;
>> +
> 
> Does this preclude something like:
> 
> modprobe ahci delete_id=1234:5678?
> 

It does seem like it would. I can look into calling into the pci
callback for this, but val is a struct module here and I haven't figured
out the plumbing to get the [correct] driver from that.

Maybe if I enforce the format of 'modprobe ahci ahci.delete_id=' to
ensure the driver is specified (and would be required in cases with
multi-driver modules).

Re: [RFC 2/3] module: Ignore delete_id parameter

[Jon Derrick]

On 09/28/2017 03:02 AM, Greg Kroah-Hartman wrote:
> On Wed, Sep 27, 2017 at 04:40:21PM -0400, Jon Derrick wrote:
>> The PCI driver delete_id parameter is handled in each individual driver
>> registration callback.
>>
>> Signed-off-by: Jon Derrick 
>> ---
>>  kernel/module.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index de66ec8..2b2dccf 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
>> *val, const char *modname,
>>  return 0;
>>  }
>>  
>> +/*
>> + * Ignore driver delete list arguments. They are handled by driver
>> + * registration callbacks
>> + */
>> +if (strcmp(param, "delete_id") == 0)
>> +return 0;
> 
> Why?  This is only for the PCI core as you have defined it in this
> patchset, but you just broke this module id for all other kernel modules
> in the system :(
> 
> If you want to do this, you need to provide this feature for _all_
> kernel drivers...
> 
> thanks,
> 
> greg k-h
> 
Yes I'm not particularly happy about this one either. I will make this
more robust if the blacklisting idea is sound.

Re: [RFC 2/3] module: Ignore delete_id parameter

[Jon Derrick]

On 09/28/2017 03:02 AM, Greg Kroah-Hartman wrote:
> On Wed, Sep 27, 2017 at 04:40:21PM -0400, Jon Derrick wrote:
>> The PCI driver delete_id parameter is handled in each individual driver
>> registration callback.
>>
>> Signed-off-by: Jon Derrick 
>> ---
>>  kernel/module.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index de66ec8..2b2dccf 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
>> *val, const char *modname,
>>  return 0;
>>  }
>>  
>> +/*
>> + * Ignore driver delete list arguments. They are handled by driver
>> + * registration callbacks
>> + */
>> +if (strcmp(param, "delete_id") == 0)
>> +return 0;
> 
> Why?  This is only for the PCI core as you have defined it in this
> patchset, but you just broke this module id for all other kernel modules
> in the system :(
> 
> If you want to do this, you need to provide this feature for _all_
> kernel drivers...
> 
> thanks,
> 
> greg k-h
> 
Yes I'm not particularly happy about this one either. I will make this
more robust if the blacklisting idea is sound.

Re: [RFC 2/3] module: Ignore delete_id parameter

[Greg Kroah-Hartman]

On Wed, Sep 27, 2017 at 04:40:21PM -0400, Jon Derrick wrote:
> The PCI driver delete_id parameter is handled in each individual driver
> registration callback.
> 
> Signed-off-by: Jon Derrick 
> ---
>  kernel/module.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index de66ec8..2b2dccf 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
> *val, const char *modname,
>   return 0;
>   }
>  
> + /*
> +  * Ignore driver delete list arguments. They are handled by driver
> +  * registration callbacks
> +  */
> + if (strcmp(param, "delete_id") == 0)
> + return 0;

Why?  This is only for the PCI core as you have defined it in this
patchset, but you just broke this module id for all other kernel modules
in the system :(

If you want to do this, you need to provide this feature for _all_
kernel drivers...

thanks,

greg k-h

Re: [RFC 2/3] module: Ignore delete_id parameter

[Greg Kroah-Hartman]

On Wed, Sep 27, 2017 at 04:40:21PM -0400, Jon Derrick wrote:
> The PCI driver delete_id parameter is handled in each individual driver
> registration callback.
> 
> Signed-off-by: Jon Derrick 
> ---
>  kernel/module.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index de66ec8..2b2dccf 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
> *val, const char *modname,
>   return 0;
>   }
>  
> + /*
> +  * Ignore driver delete list arguments. They are handled by driver
> +  * registration callbacks
> +  */
> + if (strcmp(param, "delete_id") == 0)
> + return 0;

Why?  This is only for the PCI core as you have defined it in this
patchset, but you just broke this module id for all other kernel modules
in the system :(

If you want to do this, you need to provide this feature for _all_
kernel drivers...

thanks,

greg k-h

Re: [RFC 2/3] module: Ignore delete_id parameter

[Dan Williams]

On Wed, Sep 27, 2017 at 1:40 PM, Jon Derrick  wrote:
> The PCI driver delete_id parameter is handled in each individual driver
> registration callback.
>
> Signed-off-by: Jon Derrick 
> ---
>  kernel/module.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index de66ec8..2b2dccf 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
> *val, const char *modname,
> return 0;
> }
>
> +   /*
> +* Ignore driver delete list arguments. They are handled by driver
> +* registration callbacks
> +*/
> +   if (strcmp(param, "delete_id") == 0)
> +   return 0;
> +

Does this preclude something like:

modprobe ahci delete_id=1234:5678?

Re: [RFC 2/3] module: Ignore delete_id parameter

[Dan Williams]

On Wed, Sep 27, 2017 at 1:40 PM, Jon Derrick  wrote:
> The PCI driver delete_id parameter is handled in each individual driver
> registration callback.
>
> Signed-off-by: Jon Derrick 
> ---
>  kernel/module.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index de66ec8..2b2dccf 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
> *val, const char *modname,
> return 0;
> }
>
> +   /*
> +* Ignore driver delete list arguments. They are handled by driver
> +* registration callbacks
> +*/
> +   if (strcmp(param, "delete_id") == 0)
> +   return 0;
> +

Does this preclude something like:

modprobe ahci delete_id=1234:5678?

[RFC 2/3] module: Ignore delete_id parameter

[Jon Derrick]

The PCI driver delete_id parameter is handled in each individual driver
registration callback.

Signed-off-by: Jon Derrick 
---
 kernel/module.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/kernel/module.c b/kernel/module.c
index de66ec8..2b2dccf 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
*val, const char *modname,
return 0;
}
 
+   /*
+* Ignore driver delete list arguments. They are handled by driver
+* registration callbacks
+*/
+   if (strcmp(param, "delete_id") == 0)
+   return 0;
+
/* Check for magic 'dyndbg' arg */
ret = ddebug_dyndbg_module_param_cb(param, val, modname);
if (ret != 0)
-- 
1.8.3.1

[RFC 2/3] module: Ignore delete_id parameter

[Jon Derrick]

The PCI driver delete_id parameter is handled in each individual driver
registration callback.

Signed-off-by: Jon Derrick 
---
 kernel/module.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/kernel/module.c b/kernel/module.c
index de66ec8..2b2dccf 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char 
*val, const char *modname,
return 0;
}
 
+   /*
+* Ignore driver delete list arguments. They are handled by driver
+* registration callbacks
+*/
+   if (strcmp(param, "delete_id") == 0)
+   return 0;
+
/* Check for magic 'dyndbg' arg */
ret = ddebug_dyndbg_module_param_cb(param, val, modname);
if (ret != 0)
-- 
1.8.3.1