| Message ID | c7d6a8d6-ec82-e57a-632d-dcea643ce9ab@kernel.dk (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | sg: remove bad blk_end_request_all() call | expand |
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 <dgilbert@interlog.com> > Cc: Doug Gilbert <dgilbert@interlog.com> > Cc: linux-scsi@vger.kernel.org > Signed-off-by: Jens Axboe <axboe@kernel.dk> > --- > 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; > } > >
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.
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; }
We just need to free the request here. Additionally, this is currently wrong for a queue that's using MQ currently, it'll crash. Cc: Doug Gilbert <dgilbert@interlog.com> Cc: linux-scsi@vger.kernel.org Signed-off-by: Jens Axboe <axboe@kernel.dk> --- drivers/scsi/sg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)