Re: [PATCH 3/3] null_blk: validated the number of devices

[Jens Axboe <axboe@kernel.dk>]

On 9/13/19 9:12 AM, Matthew Wilcox wrote:
> On Fri, Sep 13, 2019 at 11:57:17AM -0300, André Almeida wrote:
>> On 9/12/19 7:20 PM, Chaitanya Kulkarni wrote:
>>> On 09/12/2019 03:09 PM, André Almeida wrote:
>>>> Hello Matthew,
>>>>
>>>> On 9/12/19 1:19 PM, Matthew Wilcox wrote:
>>>>> On Wed, Sep 11, 2019 at 11:46:36AM -0300, André Almeida wrote:
>>>>>>
>>>>>> -static int nr_devices = 1;
>>>>>> +static unsigned int nr_devices = 1;
>>>>>>    module_param(nr_devices, int, 0444);
>>>>>
>>>>> ^^^ you forgot to change the module_param to match
>>>>>
>>>>>> +	if (!nr_devices) {
>>>>>> +		pr_err("null_blk: invalid number of devices\n");
>>>>>> +		return -EINVAL;
>>>>>> +	}
>>>>>
>>>>> I don't think this is necessary.
>>>>>
>>>>
>>>> Could you explain why you don't think is necessary? As I see, the module
>>>> can't be used without any /dev/nullb* device, so why we should load it?
>>>>
>>>> Thanks,
>>>> 	André
>>>>
>>>
>>> I think Matthew is right here. I think module can be loaded with
>>> nr_devices=0.
>>>
>>> Did you get a chance to test nr_device=0 condition ?
>>>
>>
>> Yes. It says "module loaded" and lsmod shows that it is loaded indeed.
>> But you don't have any /dev/nullb*, so you can't do much with the module.
>>
>> With this patch, it refuses to load the module.
> 
> Why is that an improvement?  I agree it's an uninteresting thing to ask
> for, but I don't see a compelling need to fail the module load because
> of it.

It also breaks the case of loading it, then setting up a new device
through configfs.

-- 
Jens Axboe