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@xxxxxxx>
>>> Signed-off-by: Nilay Shroff <nilay@xxxxxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxx/
Thanks,
--Nilay
