On Mon, Mar 16, 2020 at 09:02:27AM -0700, Keith Busch wrote:
> On Mon, Mar 16, 2020 at 11:30:33PM +0800, Ming Lei wrote:
> > On Mon, Mar 16, 2020 at 08:26:35PM +0800, Yufen Yu wrote:
> > > > + blk_mq_start_request(req);
> > > > +
> > > > if (req->cmd_flags & REQ_FUA)
> > > > nbd_cmd_flags |= NBD_CMD_FLAG_FUA;
> > > > @@ -879,7 +881,6 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index)
> > > > if (!refcount_inc_not_zero(&nbd->config_refs)) {
> > > > dev_err_ratelimited(disk_to_dev(nbd->disk),
> > > > "Socks array is empty\n");
> > > > - blk_mq_start_request(req);
> >
> > I think it is fine to not start request in case of failure, given
> > __blk_mq_end_request() doesn't check rq's state.
>
> Not only is it fine to not start it, blk-mq expects the low level
> driver will not tell it to start a request that the lld doesn't
> actually start.
Yeah, in theory, driver should do in this way.
> A started request should be completed through
> blk_mq_complete_request(). Returning an error from your queue_rq()
> doesn't do that, and starting it will have blk-mq track the request as
> an in-flight request.
However, error still can happen when lld is starting to queue the command
to hardware, and there are lots of such usage in drivers. I guess this
way won't be avoided completely.
Thanks,
Ming
