On 11/30/20 3:46 AM, Bart Van Assche wrote:
scsi_eh_lock_door() is the only function in the SCSI error handler that
calls blk_get_request(). It is not guaranteed that a request is available
when scsi_eh_lock_door() is called. Hence pass the BLK_MQ_REQ_NOWAIT flag
to blk_get_request().
Reviewed-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Cc: Can Guo <cang@xxxxxxxxxxxxxx>
Cc: Stanley Chu <stanley.chu@xxxxxxxxxxxx>
Cc: Ming Lei <ming.lei@xxxxxxxxxx>
Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
---
drivers/scsi/scsi_error.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index d94449188270..6de6e1bf3dcb 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1993,7 +1993,12 @@ static void scsi_eh_lock_door(struct scsi_device *sdev)
struct request *req;
struct scsi_request *rq;
- req = blk_get_request(sdev->request_queue, REQ_OP_SCSI_IN, 0);
+ /*
+ * It is not guaranteed that a request is available nor that
+ * sdev->request_queue is unfrozen. Hence the BLK_MQ_REQ_NOWAIT below.
+ */
+ req = blk_get_request(sdev->request_queue, REQ_OP_SCSI_IN,
+ BLK_MQ_REQ_NOWAIT);
if (IS_ERR(req))
return;
rq = scsi_req(req);
Well ... had been thinking about that one, too.
The idea of this function is that prior to SCSI EH the device was locked
via scsi_set_medium_removal(). And during SCSI EH the device might have
become unlocked, so we need to lock it again.
However, scsi_set_medium_removal() not only issues the
PREVENT_ALLOW_MEDIUM_REMOVAL command, but also sets the 'locked' flag
based on the result.
So if we fail to get a request here, shouldn't we unset the 'locked'
flag, too?
And what does happen if we fail here? There is no return value, hence
SCSI EH might run to completion, and the system will continue
with an unlocked door ...
Not sure if that's a good idea.
But anyway, at the very least unset the 'locked' flag upon failure such
that the internal state is correctly updated.
_Actually_, the flag should be unset after each successful SCSI EH step,
to mirror the actual state. But this is probably out of scope for this
patch.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
