On 7/3/2017 3:03 PM, Ming Lei wrote:

On Mon, Jul 03, 2017 at 01:07:44PM +0300, Sagi Grimberg wrote:

quoted

Hi Ming,

quoted

Yeah, the above change is correct, for any canceling requests in this
way we should use blk_mq_quiesce_queue().

I still don't understand why should blk_mq_flush_busy_ctxs hit a NULL
deref if we don't touch the tagset...

Looks no one mentioned the steps for reproduction, then it isn't easy
to understand the related use case, could anyone share the steps for
reproduction?

Hi Ming,
I create 500 ns per 1 subsystem (using with CX4 target and C-IB 
initiator but also saw it in CX5 vs. CX5 setup).
The null deref happens when I remove all configuration in the target (1 
port 1 subsystem and 500 namespaces and nvmet modules unload) during 
traffic to 1 nvme device/ns from the intiator.
I get Null deref in blk_mq_flush_busy_ctxs function that calls 
sbitmap_for_each_set in the initiator. seems like the "struct 
sbitmap_word *word = &sb->map[i];" is null. It's actually might be not 
null in the beginning of the func and become null during running the 
while loop there.

quoted

Also, I'm wandering in what case we shouldn't use
blk_mq_quiesce_queue()? Maybe we should unexport blk_mq_stop_hw_queues()
and blk_mq_start_stopped_hw_queues() and use the quiesce/unquiesce
equivalent always?

There are at least one case in which we have to use stop queues:

	- when QUEUE_BUSY(now it becomes BLK_STS_RESOURCE) happens, some drivers
	need to stop queues for avoiding to hurt CPU, such as virtio-blk, ...

quoted

The only fishy usage is in nvme_fc_start_fcp_op() where if submission
failed the code stop the hw queues and delays it, but I think it should
be handled differently..

It looks like the old way of scsi-mq, but scsi has removed this way and
avoids to stop queue.


Thanks,
Ming