Re: [PATCH] sg: remove bad blk_end_request_all() call

2018-10-16 Thread Martin K. Petersen


Jens,

> We just need to free the request here. Additionally, this is
> currently wrong for a queue that's using MQ currently, it'll
> crash.

Applied to 4.20/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering


Re: [PATCH] sg: remove bad blk_end_request_all() call

2018-10-16 Thread Douglas Gilbert

On 2018-10-16 10:38 a.m., Jens Axboe wrote:

We just need to free the request here. Additionally, this is
currently wrong for a queue that's using MQ currently, it'll
crash.


Surprise removals are difficult code paths to check. That snippet
is after the request has been generated and before the call to:
  blk_execute_rq_nowait()

IOWs a small window.

Currently if a surprise removal is detected by the completion
callback (sg_rq_end_io() ) then a pr_info() is called and it
otherwise follows the main path out: free-ing the request,
keeping the bio. Is that okay?

More generally, once a request is "in flight" and a surprise
removal occurs, can the sg driver expect the block layer (or
scsi mid-level) to react (and clean-up) or is it up to the
sg driver to force those actions?

Acked-by: Douglas Gilbert 


Cc: Doug Gilbert 
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Jens Axboe 
---
  drivers/scsi/sg.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 8a254bb46a9b..c6ad00703c5b 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -822,7 +822,7 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp,
if (atomic_read(&sdp->detaching)) {
if (srp->bio) {
scsi_req_free_cmd(scsi_req(srp->rq));
-   blk_end_request_all(srp->rq, BLK_STS_IOERR);
+   blk_put_request(srp->rq);
srp->rq = NULL;
}