Re: [PATCH v4 5/9] scsi: Do not wait for a request in scsi_eh_lock_door()
From: Hannes Reinecke <hare@suse.de>
Date: 2020-12-02 07:06:54
Also in:
linux-scsi
On 11/30/20 3:46 AM, Bart Van Assche wrote:
quoted hunk ↗ jump to hunk
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@rowland.harvard.edu> Reviewed-by: Christoph Hellwig <hch@lst.de> Cc: Can Guo <redacted> Cc: Stanley Chu <redacted> Cc: Ming Lei <redacted> Cc: Rafael J. Wysocki <redacted> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- 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@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer