Re: [PATCH] nbd: make starting request more reasonable

[Ming Lei <ming.lei@redhat.com>]

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