Re: [PATCH v4 4/4] scsi: core: Call blk_mq_free_tag_set() earlier

[Bart Van Assche <bvanassche@acm.org>]

On 7/13/22 01:13, John Garry wrote:
> It seems to me that blk_mq_free_tag_set() is called from 
> scsi_remove_host() now, right?

Right, I will update the patch description. I moved the 
blk_mq_free_tag_set() after I wrote the patch description.

>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index 2aca0a838ca5..295c48fdb650 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -1990,7 +1990,10 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
>>   void scsi_mq_destroy_tags(struct Scsi_Host *shost)
>>   {
>> +    if (!shost->tag_set.tags)
>> +        return;
>>       blk_mq_free_tag_set(&shost->tag_set);
>> +    WARN_ON_ONCE(shost->tag_set.tags);
> 
> blk_mq_free_tag_set() clears the tagset tags pointer, so I don't know 
> why you don't trust the semantics of that API - this seems like paranoia.

Semantics of the API? Shouldn't this rather be called an undocumented 
aspect of blk_mq_free_tag_set()?

My concern is that the "set->tags = NULL" statement might be removed in 
the future from blk_mq_free_tag_set() and also that it is possible that 
scsi_mq_destroy_tags() is not updated after that change.

Thanks,

Bart.