[PATCHv2 0/2] fix sbitmap initialization and null_blk tagset setup

[PATCHv2 0/2] fix sbitmap initialization and null_blk tagset setup

[Nilay Shroff]

Hi,

This patchset fixes two subtle issues discovered while unit testing
nr_hw_queue update code using null_blk driver.

The first patch in the series, fixes an issue in the sbitmap initialization
code, where sb->alloc_hint is not explicitly set to NULL when the sbitmap
depth is zero. This can lead to a kernel crash in sbitmap_free(), which
unconditionally calls free_percpu() on sb->alloc_hint — even if it was
never allocated. The crash is caused by dereferencing an invalid pointer
or stale garbage value.

The second patch in the series fixes a bug in the null_blk driver where
the driver_data field of the tagset is not properly initialized when
setting up shared tagsets. This omission causes null_map_queues() to fail
during nr_hw_queues update, leading to no software queues (ctx) being
mapped to new hardware queues (hctx). As a result, the affected hctx
remains unused for any IO. Interestingly, this bug exposed the first
issue with sbitmap freeing.

As usual, review and feedback are most welcome!

Changes from v1:
    - The set->driver_data field should be initialized separately for the
      shared tagset to ensure it is correctly set for both shared and
      non-shared tagset cases. (Damien Le Moal)

Nilay Shroff (2):
  lib/sbitmap: fix kernel crash observed when sbitmap depth is zero
  null_blk: fix set->driver_data while setting up tagset

 drivers/block/null_blk/main.c | 1 +
 lib/sbitmap.c                 | 1 +
 2 files changed, 2 insertions(+)

-- 
2.50.1

[PATCHv2 1/2] lib/sbitmap: fix kernel crash observed when sbitmap depth is zero

[Nilay Shroff]

We observed a kernel crash when the I/O scheduler allocates an sbitmap
for a hardware queue (hctx) that has no associated software queues (ctx),
and later attempts to free it. When no software queues are mapped to a
hardware queue, the sbitmap is initialized with a depth of zero. In such
cases, the sbitmap_init_node() function should set sb->alloc_hint to NULL.
However, if this is not done, sb->alloc_hint may contain garbage, and
calling sbitmap_free() will pass this invalid pointer to free_percpu(),
resulting in a kernel crash.

Example crash trace:
==================================================================
Kernel attempted to read user page (28) - exploit attempt? (uid: 0)
BUG: Kernel NULL pointer dereference on read at 0x00000028
Faulting instruction address: 0xc000000000708f88
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Radix  SMP NR_CPUS=2048 NUMA pSeries
[...]
CPU: 5 UID: 0 PID: 5491 Comm: mk_nullb_shared Kdump: loaded Tainted: G    B               6.16.0-rc5+ #294 VOLUNTARY
Tainted: [B]=BAD_PAGE
Hardware name: IBM,9043-MRX POWER10 (architected) 0x800200 0xf000006 of:IBM,FW1060.00 (NM1060_028) hv:phyp pSeries
[...]
NIP [c000000000708f88] free_percpu+0x144/0xba8
LR [c000000000708f84] free_percpu+0x140/0xba8
Call Trace:
    free_percpu+0x140/0xba8 (unreliable)
    kyber_exit_hctx+0x94/0x124
    blk_mq_exit_sched+0xe4/0x214
    elevator_exit+0xa8/0xf4
    elevator_switch+0x3b8/0x5d8
    elv_update_nr_hw_queues+0x14c/0x300
    blk_mq_update_nr_hw_queues+0x5cc/0x670
    nullb_update_nr_hw_queues+0x118/0x1f8 [null_blk]
    nullb_device_submit_queues_store+0xac/0x170 [null_blk]
    configfs_write_iter+0x1dc/0x2d0
    vfs_write+0x5b0/0x77c
    ksys_write+0xa0/0x180
    system_call_exception+0x1b0/0x4f0
    system_call_vectored_common+0x15c/0x2ec

If the sbitmap depth is zero, sb->alloc_hint memory is NOT allocated, but
the pointer is not explicitly set to NULL. Later, during sbitmap_free(),
the kernel attempts to free sb->alloc_hint, which is a per cpu pointer
variable, regardless of whether it was valid, leading to a crash.

This patch ensures that sb->alloc_hint is explicitly set to NULL in
sbitmap_init_node() when the requested depth is zero. This prevents
free_percpu() from freeing sb->alloc_hint and thus avoids the observed
crash.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 lib/sbitmap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/sbitmap.c b/lib/sbitmap.c
index d3412984170c..aa8b6ca76169 100644
--- a/lib/sbitmap.c
+++ b/lib/sbitmap.c
@@ -119,6 +119,7 @@ int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
 
 	if (depth == 0) {
 		sb->map = NULL;
+		sb->alloc_hint = NULL;
 		return 0;
 	}
 
-- 
2.50.1

[PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Nilay Shroff]

When setting up a null block device, we initialize a tagset that
includes a driver_data field—typically used by block drivers to
store a pointer to driver-specific data. In the case of null_blk,
this should point to the struct nullb instance.

However, due to recent tagset refactoring in the null_blk driver, we
missed initializing driver_data when creating a shared tagset. As a
result, software queues (ctx) fail to map correctly to new hardware
queues (hctx). For example, increasing the number of submit queues
triggers an nr_hw_queues update, which invokes null_map_queues() to
remap queues. Since set->driver_data is unset, null_map_queues()
fails to map any ctx to the new hctxs, leading to hctx->nr_ctx == 0,
effectively making the hardware queues unusable for I/O.

This patch fixes the issue by ensuring that set->driver_data is properly
initialized to point to the struct nullb during tagset setup.

Fixes: 72ca28765fc4 ("null_blk: refactor tag_set setup")
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/block/null_blk/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index aa163ae9b2aa..fbae0427263d 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1856,6 +1856,7 @@ static int null_setup_tagset(struct nullb *nullb)
 {
 	if (nullb->dev->shared_tags) {
 		nullb->tag_set = &tag_set;
+		nullb->tag_set->driver_data = nullb;
 		return null_init_global_tag_set();
 	}
 
-- 
2.50.1

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Damien Le Moal]

On 7/21/25 11:04 PM, Nilay Shroff wrote:
> When setting up a null block device, we initialize a tagset that
> includes a driver_data field—typically used by block drivers to
> store a pointer to driver-specific data. In the case of null_blk,
> this should point to the struct nullb instance.
> 
> However, due to recent tagset refactoring in the null_blk driver, we
> missed initializing driver_data when creating a shared tagset. As a
> result, software queues (ctx) fail to map correctly to new hardware
> queues (hctx). For example, increasing the number of submit queues
> triggers an nr_hw_queues update, which invokes null_map_queues() to
> remap queues. Since set->driver_data is unset, null_map_queues()
> fails to map any ctx to the new hctxs, leading to hctx->nr_ctx == 0,
> effectively making the hardware queues unusable for I/O.
> 
> This patch fixes the issue by ensuring that set->driver_data is properly
> initialized to point to the struct nullb during tagset setup.
> 
> Fixes: 72ca28765fc4 ("null_blk: refactor tag_set setup")
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
>  drivers/block/null_blk/main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index aa163ae9b2aa..fbae0427263d 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -1856,6 +1856,7 @@ static int null_setup_tagset(struct nullb *nullb)
>  {
>  	if (nullb->dev->shared_tags) {
>  		nullb->tag_set = &tag_set;
> +		nullb->tag_set->driver_data = nullb;

This looks better, but in the end, why is this even needed ? Since this is a
shared tagset, multiple nullb devices can use that same tagset, so setting the
driver_data pointer to this device only seems incorrect.

Checking the code, the only function that makes use of this pointer is
null_map_queues(), which correctly test for private_data being NULL.

So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?

>  		return null_init_global_tag_set();
>  	}
>  


-- 
Damien Le Moal
Western Digital Research

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Yu Kuai]

Hi,

在 2025/07/22 9:07, Damien Le Moal 写道:
> On 7/21/25 11:04 PM, Nilay Shroff wrote:
>> When setting up a null block device, we initialize a tagset that
>> includes a driver_data field—typically used by block drivers to
>> store a pointer to driver-specific data. In the case of null_blk,
>> this should point to the struct nullb instance.
>>
>> However, due to recent tagset refactoring in the null_blk driver, we
>> missed initializing driver_data when creating a shared tagset. As a
>> result, software queues (ctx) fail to map correctly to new hardware
>> queues (hctx). For example, increasing the number of submit queues
>> triggers an nr_hw_queues update, which invokes null_map_queues() to
>> remap queues. Since set->driver_data is unset, null_map_queues()
>> fails to map any ctx to the new hctxs, leading to hctx->nr_ctx == 0,
>> effectively making the hardware queues unusable for I/O.

I don't get it, for the case shared tagset, g_submit_queues and
g_poll_queues should be used, how can you increasing submit_queues?
>>
>> This patch fixes the issue by ensuring that set->driver_data is properly
>> initialized to point to the struct nullb during tagset setup.
>>
>> Fixes: 72ca28765fc4 ("null_blk: refactor tag_set setup")
>> Reviewed-by: Hannes Reinecke <hare@suse.de>
>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>> ---
>>   drivers/block/null_blk/main.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>> index aa163ae9b2aa..fbae0427263d 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -1856,6 +1856,7 @@ static int null_setup_tagset(struct nullb *nullb)
>>   {
>>   	if (nullb->dev->shared_tags) {
>>   		nullb->tag_set = &tag_set;
>> +		nullb->tag_set->driver_data = nullb;
> 
> This looks better, but in the end, why is this even needed ? Since this is a
> shared tagset, multiple nullb devices can use that same tagset, so setting the
> driver_data pointer to this device only seems incorrect.

Yes this looks incorrect, if there are multiple null_dev shared one
tag_set and blk_mq_update_nr_hw_queues() is triggered, all null_dev will
end up accesing the same null_dev in the map_queues callback.
> 
> Checking the code, the only function that makes use of this pointer is
> null_map_queues(), which correctly test for private_data being NULL.
> 
> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?

Same question :)

Thanks,
Kuai

> 
>>   		return null_init_global_tag_set();
>>   	}
>>   
> 
> 

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Nilay Shroff]

On 7/22/25 8:08 AM, Yu Kuai wrote:
> Hi,
> 
> 在 2025/07/22 9:07, Damien Le Moal 写道:
>> On 7/21/25 11:04 PM, Nilay Shroff wrote:
>>> When setting up a null block device, we initialize a tagset that
>>> includes a driver_data field—typically used by block drivers to
>>> store a pointer to driver-specific data. In the case of null_blk,
>>> this should point to the struct nullb instance.
>>>
>>> However, due to recent tagset refactoring in the null_blk driver, we
>>> missed initializing driver_data when creating a shared tagset. As a
>>> result, software queues (ctx) fail to map correctly to new hardware
>>> queues (hctx). For example, increasing the number of submit queues
>>> triggers an nr_hw_queues update, which invokes null_map_queues() to
>>> remap queues. Since set->driver_data is unset, null_map_queues()
>>> fails to map any ctx to the new hctxs, leading to hctx->nr_ctx == 0,
>>> effectively making the hardware queues unusable for I/O.
> 
> I don't get it, for the case shared tagset, g_submit_queues and
> g_poll_queues should be used, how can you increasing submit_queues?
>>>
>>> This patch fixes the issue by ensuring that set->driver_data is properly
>>> initialized to point to the struct nullb during tagset setup.
>>>
>>> Fixes: 72ca28765fc4 ("null_blk: refactor tag_set setup")
>>> Reviewed-by: Hannes Reinecke <hare@suse.de>
>>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>>> ---
>>>   drivers/block/null_blk/main.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>> index aa163ae9b2aa..fbae0427263d 100644
>>> --- a/drivers/block/null_blk/main.c
>>> +++ b/drivers/block/null_blk/main.c
>>> @@ -1856,6 +1856,7 @@ static int null_setup_tagset(struct nullb *nullb)
>>>   {
>>>       if (nullb->dev->shared_tags) {
>>>           nullb->tag_set = &tag_set;
>>> +        nullb->tag_set->driver_data = nullb;
>>
>> This looks better, but in the end, why is this even needed ? Since this is a
>> shared tagset, multiple nullb devices can use that same tagset, so setting the
>> driver_data pointer to this device only seems incorrect.
> 
> Yes this looks incorrect, if there are multiple null_dev shared one
> tag_set and blk_mq_update_nr_hw_queues() is triggered, all null_dev will
> end up accesing the same null_dev in the map_queues callback.
>>
>> Checking the code, the only function that makes use of this pointer is
>> null_map_queues(), which correctly test for private_data being NULL.
>>
>> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?
> 
> Same question :)

I have responded to Damien in another thread. Sorry I missed to add you in that thread... 
You may refer the thread here: https://lore.kernel.org/all/3c33c551-b707-4fd2-bd52-418ff3bc547c@linux.ibm.com/

Thanks,
--Nilay

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Nilay Shroff]

On 7/22/25 6:37 AM, Damien Le Moal wrote:
> On 7/21/25 11:04 PM, Nilay Shroff wrote:
>> When setting up a null block device, we initialize a tagset that
>> includes a driver_data field—typically used by block drivers to
>> store a pointer to driver-specific data. In the case of null_blk,
>> this should point to the struct nullb instance.
>>
>> However, due to recent tagset refactoring in the null_blk driver, we
>> missed initializing driver_data when creating a shared tagset. As a
>> result, software queues (ctx) fail to map correctly to new hardware
>> queues (hctx). For example, increasing the number of submit queues
>> triggers an nr_hw_queues update, which invokes null_map_queues() to
>> remap queues. Since set->driver_data is unset, null_map_queues()
>> fails to map any ctx to the new hctxs, leading to hctx->nr_ctx == 0,
>> effectively making the hardware queues unusable for I/O.
>>
>> This patch fixes the issue by ensuring that set->driver_data is properly
>> initialized to point to the struct nullb during tagset setup.
>>
>> Fixes: 72ca28765fc4 ("null_blk: refactor tag_set setup")
>> Reviewed-by: Hannes Reinecke <hare@suse.de>
>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>> ---
>>  drivers/block/null_blk/main.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>> index aa163ae9b2aa..fbae0427263d 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -1856,6 +1856,7 @@ static int null_setup_tagset(struct nullb *nullb)
>>  {
>>  	if (nullb->dev->shared_tags) {
>>  		nullb->tag_set = &tag_set;
>> +		nullb->tag_set->driver_data = nullb;
> 
> This looks better, but in the end, why is this even needed ? Since this is a
> shared tagset, multiple nullb devices can use that same tagset, so setting the
> driver_data pointer to this device only seems incorrect.
> 
> Checking the code, the only function that makes use of this pointer is
> null_map_queues(), which correctly test for private_data being NULL.
> 
> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?
> 
I think you were right — the first patch alone is sufficient to prevent the crash.
Initially, I thought the second patch might also be necessary, especially for the
scenario where the user increases the number of submit_queues for a null_blk device.
While the block layer does create a new hardware queue (hctx) in response to such
an update, null_blk (null_map_queues()) does not map any software queue (ctx) to it. 
As a result, the newly added hctx remains unused for I/O.

Given this behavior, I believe we should not allow users to update submit_queues 
on a null_blk device if it is using a shared tagset. Currently, the code allows
this update, but it’s misleading since the change has no actual effect.

Would it make sense to explicitly prevent updating submit_queues in this case?
That would align the interface with the actual behavior and avoid potential
confusion and also saves us allocating memory for hctx which remains unused.
Maybe we should have this check, in nullb_update_nr_hw_queues():

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index fbae0427263d..aed2a6904db1 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -388,6 +388,12 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
        if (!submit_queues)
                return -EINVAL;
 
+       /*
+        * Cannot update queues with shared tagset.
+        */
+       if (dev->shared_tags)
+               return -EINVAL;
+
        /*
         * Make sure that null_init_hctx() does not access nullb->queues[] past
         * the end of that array.

If the above change looks good, maybe I can update second patch with
the above change.

Thanks,
--Nilay

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Damien Le Moal]

On 7/22/25 5:30 PM, Nilay Shroff wrote:
> 
> 
> On 7/22/25 6:37 AM, Damien Le Moal wrote:
>> On 7/21/25 11:04 PM, Nilay Shroff wrote:
>>> When setting up a null block device, we initialize a tagset that
>>> includes a driver_data field—typically used by block drivers to
>>> store a pointer to driver-specific data. In the case of null_blk,
>>> this should point to the struct nullb instance.
>>>
>>> However, due to recent tagset refactoring in the null_blk driver, we
>>> missed initializing driver_data when creating a shared tagset. As a
>>> result, software queues (ctx) fail to map correctly to new hardware
>>> queues (hctx). For example, increasing the number of submit queues
>>> triggers an nr_hw_queues update, which invokes null_map_queues() to
>>> remap queues. Since set->driver_data is unset, null_map_queues()
>>> fails to map any ctx to the new hctxs, leading to hctx->nr_ctx == 0,
>>> effectively making the hardware queues unusable for I/O.
>>>
>>> This patch fixes the issue by ensuring that set->driver_data is properly
>>> initialized to point to the struct nullb during tagset setup.
>>>
>>> Fixes: 72ca28765fc4 ("null_blk: refactor tag_set setup")
>>> Reviewed-by: Hannes Reinecke <hare@suse.de>
>>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>>> ---
>>>  drivers/block/null_blk/main.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>> index aa163ae9b2aa..fbae0427263d 100644
>>> --- a/drivers/block/null_blk/main.c
>>> +++ b/drivers/block/null_blk/main.c
>>> @@ -1856,6 +1856,7 @@ static int null_setup_tagset(struct nullb *nullb)
>>>  {
>>>  	if (nullb->dev->shared_tags) {
>>>  		nullb->tag_set = &tag_set;
>>> +		nullb->tag_set->driver_data = nullb;
>>
>> This looks better, but in the end, why is this even needed ? Since this is a
>> shared tagset, multiple nullb devices can use that same tagset, so setting the
>> driver_data pointer to this device only seems incorrect.
>>
>> Checking the code, the only function that makes use of this pointer is
>> null_map_queues(), which correctly test for private_data being NULL.
>>
>> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?
>>
> I think you were right — the first patch alone is sufficient to prevent the crash.
> Initially, I thought the second patch might also be necessary, especially for the
> scenario where the user increases the number of submit_queues for a null_blk device.
> While the block layer does create a new hardware queue (hctx) in response to such
> an update, null_blk (null_map_queues()) does not map any software queue (ctx) to it. 
> As a result, the newly added hctx remains unused for I/O.
> 
> Given this behavior, I believe we should not allow users to update submit_queues 
> on a null_blk device if it is using a shared tagset. Currently, the code allows
> this update, but it’s misleading since the change has no actual effect.
> 
> Would it make sense to explicitly prevent updating submit_queues in this case?
> That would align the interface with the actual behavior and avoid potential
> confusion and also saves us allocating memory for hctx which remains unused.
> Maybe we should have this check, in nullb_update_nr_hw_queues():
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index fbae0427263d..aed2a6904db1 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -388,6 +388,12 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
>         if (!submit_queues)
>                 return -EINVAL;
>  
> +       /*
> +        * Cannot update queues with shared tagset.
> +        */
> +       if (dev->shared_tags)
> +               return -EINVAL;
> +

I think that is fine since null_init_global_tag_set() uses g_submit_queues
(module parameter).

>         /*
>          * Make sure that null_init_hctx() does not access nullb->queues[] past
>          * the end of that array.
> 
> If the above change looks good, maybe I can update second patch with
> the above change.
> 
> Thanks,
> --Nilay
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Yu Kuai]

Hi,

在 2025/07/22 16:30, Nilay Shroff 写道:
> 
> 
> On 7/22/25 6:37 AM, Damien Le Moal wrote:
>> On 7/21/25 11:04 PM, Nilay Shroff wrote:
>>> When setting up a null block device, we initialize a tagset that
>>> includes a driver_data field—typically used by block drivers to
>>> store a pointer to driver-specific data. In the case of null_blk,
>>> this should point to the struct nullb instance.
>>>
>>> However, due to recent tagset refactoring in the null_blk driver, we
>>> missed initializing driver_data when creating a shared tagset. As a
>>> result, software queues (ctx) fail to map correctly to new hardware
>>> queues (hctx). For example, increasing the number of submit queues
>>> triggers an nr_hw_queues update, which invokes null_map_queues() to
>>> remap queues. Since set->driver_data is unset, null_map_queues()
>>> fails to map any ctx to the new hctxs, leading to hctx->nr_ctx == 0,
>>> effectively making the hardware queues unusable for I/O.
>>>
>>> This patch fixes the issue by ensuring that set->driver_data is properly
>>> initialized to point to the struct nullb during tagset setup.
>>>
>>> Fixes: 72ca28765fc4 ("null_blk: refactor tag_set setup")
>>> Reviewed-by: Hannes Reinecke <hare@suse.de>
>>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>>> ---
>>>   drivers/block/null_blk/main.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>> index aa163ae9b2aa..fbae0427263d 100644
>>> --- a/drivers/block/null_blk/main.c
>>> +++ b/drivers/block/null_blk/main.c
>>> @@ -1856,6 +1856,7 @@ static int null_setup_tagset(struct nullb *nullb)
>>>   {
>>>   	if (nullb->dev->shared_tags) {
>>>   		nullb->tag_set = &tag_set;
>>> +		nullb->tag_set->driver_data = nullb;
>>
>> This looks better, but in the end, why is this even needed ? Since this is a
>> shared tagset, multiple nullb devices can use that same tagset, so setting the
>> driver_data pointer to this device only seems incorrect.
>>
>> Checking the code, the only function that makes use of this pointer is
>> null_map_queues(), which correctly test for private_data being NULL.
>>
>> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?
>>
> I think you were right — the first patch alone is sufficient to prevent the crash.
> Initially, I thought the second patch might also be necessary, especially for the
> scenario where the user increases the number of submit_queues for a null_blk device.
> While the block layer does create a new hardware queue (hctx) in response to such
> an update, null_blk (null_map_queues()) does not map any software queue (ctx) to it.
> As a result, the newly added hctx remains unused for I/O.
> 
> Given this behavior, I believe we should not allow users to update submit_queues
> on a null_blk device if it is using a shared tagset. Currently, the code allows
> this update, but it’s misleading since the change has no actual effect.
> 
> Would it make sense to explicitly prevent updating submit_queues in this case?
> That would align the interface with the actual behavior and avoid potential
> confusion and also saves us allocating memory for hctx which remains unused.
> Maybe we should have this check, in nullb_update_nr_hw_queues():
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index fbae0427263d..aed2a6904db1 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -388,6 +388,12 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
>          if (!submit_queues)
>                  return -EINVAL;
>   
> +       /*
> +        * Cannot update queues with shared tagset.
> +        */
> +       if (dev->shared_tags)
> +               return -EINVAL;
> +
>          /*
>           * Make sure that null_init_hctx() does not access nullb->queues[] past
>           * the end of that array.
> 
> If the above change looks good, maybe I can update second patch with
> the above change.

I'm good with this change, howerver, with a quick look it seems lots of
configfs api are broken for shared_tags. For example:

If we switch shared_tags from 0 to 1, and then read submit_queues,
looks like it's the old dev->submit_queues instead of g_submit_queues;

Thanks,
Kuai

> 
> Thanks,
> --Nilay
> 
> 
> 
> .
> 

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Damien Le Moal]

On 7/22/25 6:17 PM, Yu Kuai wrote:
> Hi,
> 
> 在 2025/07/22 16:30, Nilay Shroff 写道:
>>
>>
>> On 7/22/25 6:37 AM, Damien Le Moal wrote:
>>> On 7/21/25 11:04 PM, Nilay Shroff wrote:
>>>> When setting up a null block device, we initialize a tagset that
>>>> includes a driver_data field—typically used by block drivers to
>>>> store a pointer to driver-specific data. In the case of null_blk,
>>>> this should point to the struct nullb instance.
>>>>
>>>> However, due to recent tagset refactoring in the null_blk driver, we
>>>> missed initializing driver_data when creating a shared tagset. As a
>>>> result, software queues (ctx) fail to map correctly to new hardware
>>>> queues (hctx). For example, increasing the number of submit queues
>>>> triggers an nr_hw_queues update, which invokes null_map_queues() to
>>>> remap queues. Since set->driver_data is unset, null_map_queues()
>>>> fails to map any ctx to the new hctxs, leading to hctx->nr_ctx == 0,
>>>> effectively making the hardware queues unusable for I/O.
>>>>
>>>> This patch fixes the issue by ensuring that set->driver_data is properly
>>>> initialized to point to the struct nullb during tagset setup.
>>>>
>>>> Fixes: 72ca28765fc4 ("null_blk: refactor tag_set setup")
>>>> Reviewed-by: Hannes Reinecke <hare@suse.de>
>>>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>>>> ---
>>>>   drivers/block/null_blk/main.c | 1 +
>>>>   1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>>> index aa163ae9b2aa..fbae0427263d 100644
>>>> --- a/drivers/block/null_blk/main.c
>>>> +++ b/drivers/block/null_blk/main.c
>>>> @@ -1856,6 +1856,7 @@ static int null_setup_tagset(struct nullb *nullb)
>>>>   {
>>>>       if (nullb->dev->shared_tags) {
>>>>           nullb->tag_set = &tag_set;
>>>> +        nullb->tag_set->driver_data = nullb;
>>>
>>> This looks better, but in the end, why is this even needed ? Since this is a
>>> shared tagset, multiple nullb devices can use that same tagset, so setting the
>>> driver_data pointer to this device only seems incorrect.
>>>
>>> Checking the code, the only function that makes use of this pointer is
>>> null_map_queues(), which correctly test for private_data being NULL.
>>>
>>> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?
>>>
>> I think you were right — the first patch alone is sufficient to prevent the
>> crash.
>> Initially, I thought the second patch might also be necessary, especially for
>> the
>> scenario where the user increases the number of submit_queues for a null_blk
>> device.
>> While the block layer does create a new hardware queue (hctx) in response to
>> such
>> an update, null_blk (null_map_queues()) does not map any software queue (ctx)
>> to it.
>> As a result, the newly added hctx remains unused for I/O.
>>
>> Given this behavior, I believe we should not allow users to update submit_queues
>> on a null_blk device if it is using a shared tagset. Currently, the code allows
>> this update, but it’s misleading since the change has no actual effect.
>>
>> Would it make sense to explicitly prevent updating submit_queues in this case?
>> That would align the interface with the actual behavior and avoid potential
>> confusion and also saves us allocating memory for hctx which remains unused.
>> Maybe we should have this check, in nullb_update_nr_hw_queues():
>>
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>> index fbae0427263d..aed2a6904db1 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -388,6 +388,12 @@ static int nullb_update_nr_hw_queues(struct nullb_device
>> *dev,
>>          if (!submit_queues)
>>                  return -EINVAL;
>>   +       /*
>> +        * Cannot update queues with shared tagset.
>> +        */
>> +       if (dev->shared_tags)
>> +               return -EINVAL;
>> +
>>          /*
>>           * Make sure that null_init_hctx() does not access nullb->queues[] past
>>           * the end of that array.
>>
>> If the above change looks good, maybe I can update second patch with
>> the above change.
> 
> I'm good with this change, howerver, with a quick look it seems lots of
> configfs api are broken for shared_tags. For example:
> 
> If we switch shared_tags from 0 to 1, and then read submit_queues,
> looks like it's the old dev->submit_queues instead of g_submit_queues;

g_submit_queues is used as the initial value for dev->submit_queues. See
nullb_alloc_dev(). So if we prevent changing dev->submit_queues with configfs,
we'll get whatever g_submit_queues was set on modprobe for the shared tagset.



-- 
Damien Le Moal
Western Digital Research

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Yu Kuai]

Hi,

在 2025/07/22 17:18, Damien Le Moal 写道:
> On 7/22/25 6:17 PM, Yu Kuai wrote:
>> Hi,
>>
>> 在 2025/07/22 16:30, Nilay Shroff 写道:
>>>
>>>
>>> On 7/22/25 6:37 AM, Damien Le Moal wrote:
>>>> On 7/21/25 11:04 PM, Nilay Shroff wrote:
>>>>> When setting up a null block device, we initialize a tagset that
>>>>> includes a driver_data field—typically used by block drivers to
>>>>> store a pointer to driver-specific data. In the case of null_blk,
>>>>> this should point to the struct nullb instance.
>>>>>
>>>>> However, due to recent tagset refactoring in the null_blk driver, we
>>>>> missed initializing driver_data when creating a shared tagset. As a
>>>>> result, software queues (ctx) fail to map correctly to new hardware
>>>>> queues (hctx). For example, increasing the number of submit queues
>>>>> triggers an nr_hw_queues update, which invokes null_map_queues() to
>>>>> remap queues. Since set->driver_data is unset, null_map_queues()
>>>>> fails to map any ctx to the new hctxs, leading to hctx->nr_ctx == 0,
>>>>> effectively making the hardware queues unusable for I/O.
>>>>>
>>>>> This patch fixes the issue by ensuring that set->driver_data is properly
>>>>> initialized to point to the struct nullb during tagset setup.
>>>>>
>>>>> Fixes: 72ca28765fc4 ("null_blk: refactor tag_set setup")
>>>>> Reviewed-by: Hannes Reinecke <hare@suse.de>
>>>>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>>>>> ---
>>>>>    drivers/block/null_blk/main.c | 1 +
>>>>>    1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>>>> index aa163ae9b2aa..fbae0427263d 100644
>>>>> --- a/drivers/block/null_blk/main.c
>>>>> +++ b/drivers/block/null_blk/main.c
>>>>> @@ -1856,6 +1856,7 @@ static int null_setup_tagset(struct nullb *nullb)
>>>>>    {
>>>>>        if (nullb->dev->shared_tags) {
>>>>>            nullb->tag_set = &tag_set;
>>>>> +        nullb->tag_set->driver_data = nullb;
>>>>
>>>> This looks better, but in the end, why is this even needed ? Since this is a
>>>> shared tagset, multiple nullb devices can use that same tagset, so setting the
>>>> driver_data pointer to this device only seems incorrect.
>>>>
>>>> Checking the code, the only function that makes use of this pointer is
>>>> null_map_queues(), which correctly test for private_data being NULL.
>>>>
>>>> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?
>>>>
>>> I think you were right — the first patch alone is sufficient to prevent the
>>> crash.
>>> Initially, I thought the second patch might also be necessary, especially for
>>> the
>>> scenario where the user increases the number of submit_queues for a null_blk
>>> device.
>>> While the block layer does create a new hardware queue (hctx) in response to
>>> such
>>> an update, null_blk (null_map_queues()) does not map any software queue (ctx)
>>> to it.
>>> As a result, the newly added hctx remains unused for I/O.
>>>
>>> Given this behavior, I believe we should not allow users to update submit_queues
>>> on a null_blk device if it is using a shared tagset. Currently, the code allows
>>> this update, but it’s misleading since the change has no actual effect.
>>>
>>> Would it make sense to explicitly prevent updating submit_queues in this case?
>>> That would align the interface with the actual behavior and avoid potential
>>> confusion and also saves us allocating memory for hctx which remains unused.
>>> Maybe we should have this check, in nullb_update_nr_hw_queues():
>>>
>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>> index fbae0427263d..aed2a6904db1 100644
>>> --- a/drivers/block/null_blk/main.c
>>> +++ b/drivers/block/null_blk/main.c
>>> @@ -388,6 +388,12 @@ static int nullb_update_nr_hw_queues(struct nullb_device
>>> *dev,
>>>           if (!submit_queues)
>>>                   return -EINVAL;
>>>    +       /*
>>> +        * Cannot update queues with shared tagset.
>>> +        */
>>> +       if (dev->shared_tags)
>>> +               return -EINVAL;
>>> +
>>>           /*
>>>            * Make sure that null_init_hctx() does not access nullb->queues[] past
>>>            * the end of that array.
>>>
>>> If the above change looks good, maybe I can update second patch with
>>> the above change.
>>
>> I'm good with this change, howerver, with a quick look it seems lots of
>> configfs api are broken for shared_tags. For example:
>>
>> If we switch shared_tags from 0 to 1, and then read submit_queues,
>> looks like it's the old dev->submit_queues instead of g_submit_queues;
> 
> g_submit_queues is used as the initial value for dev->submit_queues. See
> nullb_alloc_dev(). So if we prevent changing dev->submit_queues with configfs,
> we'll get whatever g_submit_queues was set on modprobe for the shared tagset.

I know that, I mean in the case shared_tags is 0 and set submit_queues
different from g_submit_queues, and then *switch shared_tags from 0 to
1*, user will read wrong submit_queues :)

Thanks,
Kuai

> 
> 
> 

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Nilay Shroff]

On 7/22/25 2:54 PM, Yu Kuai wrote:
>>>>> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?
>>>>>
>>>> I think you were right — the first patch alone is sufficient to prevent the
>>>> crash.
>>>> Initially, I thought the second patch might also be necessary, especially for
>>>> the
>>>> scenario where the user increases the number of submit_queues for a null_blk
>>>> device.
>>>> While the block layer does create a new hardware queue (hctx) in response to
>>>> such
>>>> an update, null_blk (null_map_queues()) does not map any software queue (ctx)
>>>> to it.
>>>> As a result, the newly added hctx remains unused for I/O.
>>>>
>>>> Given this behavior, I believe we should not allow users to update submit_queues
>>>> on a null_blk device if it is using a shared tagset. Currently, the code allows
>>>> this update, but it’s misleading since the change has no actual effect.
>>>>
>>>> Would it make sense to explicitly prevent updating submit_queues in this case?
>>>> That would align the interface with the actual behavior and avoid potential
>>>> confusion and also saves us allocating memory for hctx which remains unused.
>>>> Maybe we should have this check, in nullb_update_nr_hw_queues():
>>>>
>>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>>> index fbae0427263d..aed2a6904db1 100644
>>>> --- a/drivers/block/null_blk/main.c
>>>> +++ b/drivers/block/null_blk/main.c
>>>> @@ -388,6 +388,12 @@ static int nullb_update_nr_hw_queues(struct nullb_device
>>>> *dev,
>>>>           if (!submit_queues)
>>>>                   return -EINVAL;
>>>>    +       /*
>>>> +        * Cannot update queues with shared tagset.
>>>> +        */
>>>> +       if (dev->shared_tags)
>>>> +               return -EINVAL;
>>>> +
>>>>           /*
>>>>            * Make sure that null_init_hctx() does not access nullb->queues[] past
>>>>            * the end of that array.
>>>>
>>>> If the above change looks good, maybe I can update second patch with
>>>> the above change.
>>>
>>> I'm good with this change, howerver, with a quick look it seems lots of
>>> configfs api are broken for shared_tags. For example:
>>>
>>> If we switch shared_tags from 0 to 1, and then read submit_queues,
>>> looks like it's the old dev->submit_queues instead of g_submit_queues;
>>
>> g_submit_queues is used as the initial value for dev->submit_queues. See
>> nullb_alloc_dev(). So if we prevent changing dev->submit_queues with configfs,
>> we'll get whatever g_submit_queues was set on modprobe for the shared tagset.
> 
> I know that, I mean in the case shared_tags is 0 and set submit_queues
> different from g_submit_queues, and then *switch shared_tags from 0 to
> 1*, user will read wrong submit_queues :)
> 
I think I got what you were referring here. So in addition to the above change
we also need to validate the nullb config before we power on nullb device. And
we can add that in null_validate_conf():

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index aa163ae9b2aa..1762dc541a17 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1893,6 +1893,10 @@ static int null_validate_conf(struct nullb_device *dev)
                dev->submit_queues = 1;
        dev->prev_submit_queues = dev->submit_queues;
 
+       if (dev->shared_tags)
+               if (dev->submit_queues > g_submit_queues)
+                       dev->submit_queues = g_submit_queues;
+
        if (dev->poll_queues > g_poll_queues)
                dev->poll_queues = g_poll_queues;
        dev->prev_poll_queues = dev->poll_queues;

If this looks good then I could also add the above change in this 
patchset.

Thanks,
--Nilay

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Yu Kuai]

Hi,

在 2025/07/22 18:19, Nilay Shroff 写道:
> 
> 
> On 7/22/25 2:54 PM, Yu Kuai wrote:
>>>>>> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?
>>>>>>
>>>>> I think you were right — the first patch alone is sufficient to prevent the
>>>>> crash.
>>>>> Initially, I thought the second patch might also be necessary, especially for
>>>>> the
>>>>> scenario where the user increases the number of submit_queues for a null_blk
>>>>> device.
>>>>> While the block layer does create a new hardware queue (hctx) in response to
>>>>> such
>>>>> an update, null_blk (null_map_queues()) does not map any software queue (ctx)
>>>>> to it.
>>>>> As a result, the newly added hctx remains unused for I/O.
>>>>>
>>>>> Given this behavior, I believe we should not allow users to update submit_queues
>>>>> on a null_blk device if it is using a shared tagset. Currently, the code allows
>>>>> this update, but it’s misleading since the change has no actual effect.
>>>>>
>>>>> Would it make sense to explicitly prevent updating submit_queues in this case?
>>>>> That would align the interface with the actual behavior and avoid potential
>>>>> confusion and also saves us allocating memory for hctx which remains unused.
>>>>> Maybe we should have this check, in nullb_update_nr_hw_queues():
>>>>>
>>>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>>>> index fbae0427263d..aed2a6904db1 100644
>>>>> --- a/drivers/block/null_blk/main.c
>>>>> +++ b/drivers/block/null_blk/main.c
>>>>> @@ -388,6 +388,12 @@ static int nullb_update_nr_hw_queues(struct nullb_device
>>>>> *dev,
>>>>>            if (!submit_queues)
>>>>>                    return -EINVAL;
>>>>>     +       /*
>>>>> +        * Cannot update queues with shared tagset.
>>>>> +        */
>>>>> +       if (dev->shared_tags)
>>>>> +               return -EINVAL;
>>>>> +
>>>>>            /*
>>>>>             * Make sure that null_init_hctx() does not access nullb->queues[] past
>>>>>             * the end of that array.
>>>>>
>>>>> If the above change looks good, maybe I can update second patch with
>>>>> the above change.
>>>>
>>>> I'm good with this change, howerver, with a quick look it seems lots of
>>>> configfs api are broken for shared_tags. For example:
>>>>
>>>> If we switch shared_tags from 0 to 1, and then read submit_queues,
>>>> looks like it's the old dev->submit_queues instead of g_submit_queues;
>>>
>>> g_submit_queues is used as the initial value for dev->submit_queues. See
>>> nullb_alloc_dev(). So if we prevent changing dev->submit_queues with configfs,
>>> we'll get whatever g_submit_queues was set on modprobe for the shared tagset.
>>
>> I know that, I mean in the case shared_tags is 0 and set submit_queues
>> different from g_submit_queues, and then *switch shared_tags from 0 to
>> 1*, user will read wrong submit_queues :)
>>
> I think I got what you were referring here. So in addition to the above change
> we also need to validate the nullb config before we power on nullb device. And
> we can add that in null_validate_conf():
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index aa163ae9b2aa..1762dc541a17 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -1893,6 +1893,10 @@ static int null_validate_conf(struct nullb_device *dev)
>                  dev->submit_queues = 1;
>          dev->prev_submit_queues = dev->submit_queues;
>   
> +       if (dev->shared_tags)
> +               if (dev->submit_queues > g_submit_queues)
> +                       dev->submit_queues = g_submit_queues;

Perhaps:

	if (dev->shared_tags) {
		dev->submit_queues = g_submit_queues;
		dev->poll_queues = g_poll_queues;
	} else if (dev->poll_queues > g_poll_queues) {
		dev->poll_queues = g_poll_queues;
	}

Thanks,
Kuai
> +
>          if (dev->poll_queues > g_poll_queues)
>                  dev->poll_queues = g_poll_queues;
>          dev->prev_poll_queues = dev->poll_queues;
> 
> If this looks good then I could also add the above change in this
> patchset.
> 
> Thanks,
> --Nilay
> .
> 

Re: [PATCHv2 2/2] null_blk: fix set->driver_data while setting up tagset

[Nilay Shroff]

On 7/23/25 6:11 AM, Yu Kuai wrote:
> Hi,
> 
> 在 2025/07/22 18:19, Nilay Shroff 写道:
>>
>>
>> On 7/22/25 2:54 PM, Yu Kuai wrote:
>>>>>>> So why do we need this ? Isn't your patch 1/2 enough to fix the crash you got ?
>>>>>>>
>>>>>> I think you were right — the first patch alone is sufficient to prevent the
>>>>>> crash.
>>>>>> Initially, I thought the second patch might also be necessary, especially for
>>>>>> the
>>>>>> scenario where the user increases the number of submit_queues for a null_blk
>>>>>> device.
>>>>>> While the block layer does create a new hardware queue (hctx) in response to
>>>>>> such
>>>>>> an update, null_blk (null_map_queues()) does not map any software queue (ctx)
>>>>>> to it.
>>>>>> As a result, the newly added hctx remains unused for I/O.
>>>>>>
>>>>>> Given this behavior, I believe we should not allow users to update submit_queues
>>>>>> on a null_blk device if it is using a shared tagset. Currently, the code allows
>>>>>> this update, but it’s misleading since the change has no actual effect.
>>>>>>
>>>>>> Would it make sense to explicitly prevent updating submit_queues in this case?
>>>>>> That would align the interface with the actual behavior and avoid potential
>>>>>> confusion and also saves us allocating memory for hctx which remains unused.
>>>>>> Maybe we should have this check, in nullb_update_nr_hw_queues():
>>>>>>
>>>>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>>>>> index fbae0427263d..aed2a6904db1 100644
>>>>>> --- a/drivers/block/null_blk/main.c
>>>>>> +++ b/drivers/block/null_blk/main.c
>>>>>> @@ -388,6 +388,12 @@ static int nullb_update_nr_hw_queues(struct nullb_device
>>>>>> *dev,
>>>>>>            if (!submit_queues)
>>>>>>                    return -EINVAL;
>>>>>>     +       /*
>>>>>> +        * Cannot update queues with shared tagset.
>>>>>> +        */
>>>>>> +       if (dev->shared_tags)
>>>>>> +               return -EINVAL;
>>>>>> +
>>>>>>            /*
>>>>>>             * Make sure that null_init_hctx() does not access nullb->queues[] past
>>>>>>             * the end of that array.
>>>>>>
>>>>>> If the above change looks good, maybe I can update second patch with
>>>>>> the above change.
>>>>>
>>>>> I'm good with this change, howerver, with a quick look it seems lots of
>>>>> configfs api are broken for shared_tags. For example:
>>>>>
>>>>> If we switch shared_tags from 0 to 1, and then read submit_queues,
>>>>> looks like it's the old dev->submit_queues instead of g_submit_queues;
>>>>
>>>> g_submit_queues is used as the initial value for dev->submit_queues. See
>>>> nullb_alloc_dev(). So if we prevent changing dev->submit_queues with configfs,
>>>> we'll get whatever g_submit_queues was set on modprobe for the shared tagset.
>>>
>>> I know that, I mean in the case shared_tags is 0 and set submit_queues
>>> different from g_submit_queues, and then *switch shared_tags from 0 to
>>> 1*, user will read wrong submit_queues :)
>>>
>> I think I got what you were referring here. So in addition to the above change
>> we also need to validate the nullb config before we power on nullb device. And
>> we can add that in null_validate_conf():
>>
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>> index aa163ae9b2aa..1762dc541a17 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -1893,6 +1893,10 @@ static int null_validate_conf(struct nullb_device *dev)
>>                  dev->submit_queues = 1;
>>          dev->prev_submit_queues = dev->submit_queues;
>>   +       if (dev->shared_tags)
>> +               if (dev->submit_queues > g_submit_queues)
>> +                       dev->submit_queues = g_submit_queues;
> 
> Perhaps:
> 
>     if (dev->shared_tags) {
>         dev->submit_queues = g_submit_queues;
>         dev->poll_queues = g_poll_queues;
>     } else if (dev->poll_queues > g_poll_queues) {
>         dev->poll_queues = g_poll_queues;
>     }
> 
Okay this makes sense. So for the shared tagset, we'd 
always reset the submit and poll queues value to the 
respective values set while loading the module. And for 
the non-shared tagset, we'd clamp poll_queues when its 
value exceeds the value initialized during module load
time. I will incorporate this in the next patch.

Thanks,
--Nilay