Re: +AFs-PATCH+AF0- blk-mq: Set request mapping to NULL in blk+AF8-mq+AF8-put+AF8-driver+AF8-tag

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

On Tue, 2018-12-04 at 23:48 +0530, Kashyap Desai wrote:
> Let me explain the issue. It is not a race, but very straight issue.

Please clarify what makes you think that iterating over all requests does not
race with request completion. Is request submission perhaps blocked during the
firmware recovery process? Does the firmware recovery process wait for
completion interrupts that are in progress to finish?

> Let's
> say we have one scsi_device /dev/sda and total IO submitted + completed are
> some number 100.
> All the 100 IO is *completed*.   Now, As part of Firmware recovery, driver
> tries to find our outstanding IOs using scsi_host_find_tag().
> Block layer will return all the 100 commands to the driver but really those
> 100 commands are not outstanding. This patch will return *actual*
> outstanding commands.

Would iterating over started requests be a good alternative to the patch
that you had posted? The header above blk_mq_tagset_busy_iter() is as follows
(this function is used by e.g. the skd driver for firmware recovery):

/**
 * blk_mq_tagset_busy_iter - iterate over all started requests in a tag set
 * @tagset:	Tag set to iterate over.
 * @fn:		Pointer to the function that will be called for each started
 *		request. @fn will be called as follows: @fn(rq, @priv,
 *		reserved) where rq is a pointer to a request. 'reserved'
 *		indicates whether or not @rq is a reserved request.
 * @priv:	Will be passed as second argument to @fn.
 */

> I am not able to find right context from srp, but I check the srp code and
> looks like that driver is getting scmd using scsi_host_find_tag() for live
> command.

Sorry, my e-mail was a bit too short to be comprehensible. When running
sg_reset -d /dev/sd... in a loop it is possible that the SCSI reset handler
terminates a request while the SRP driver is handling a request completion
for one of the terminated requests. That is why both srp_process_srp() and
the reset handler call srp_claim_req() to make sure that only one of these
two contexts completes a request.

Bart.