On Tue, Sep 17, 2019 at 6:46 PM Bart Van Assche <bvanassche@xxxxxxx> wrote:
>
> On 9/17/19 6:09 AM, Jinpu Wang wrote:
> >>> +static void ibnbd_softirq_done_fn(struct request *rq)
> >>> +{
> >>> + struct ibnbd_clt_dev *dev = rq->rq_disk->private_data;
> >>> + struct ibnbd_clt_session *sess = dev->sess;
> >>> + struct ibnbd_iu *iu;
> >>> +
> >>> + iu = blk_mq_rq_to_pdu(rq);
> >>> + ibnbd_put_tag(sess, iu->tag);
> >>> + blk_mq_end_request(rq, iu->status);
> >>> +}
> >>> +
> >>> +static void msg_io_conf(void *priv, int errno)
> >>> +{
> >>> + struct ibnbd_iu *iu = (struct ibnbd_iu *)priv;
> >>> + struct ibnbd_clt_dev *dev = iu->dev;
> >>> + struct request *rq = iu->rq;
> >>> +
> >>> + iu->status = errno ? BLK_STS_IOERR : BLK_STS_OK;
> >>> +
> >>> + if (softirq_enable) {
> >>> + blk_mq_complete_request(rq);
> >>> + } else {
> >>> + ibnbd_put_tag(dev->sess, iu->tag);
> >>> + blk_mq_end_request(rq, iu->status);
> >>> + }
> >>
> >> Block drivers must call blk_mq_complete_request() instead of
> >> blk_mq_end_request() to complete a request after processing of the
> >> request has been started. Calling blk_mq_end_request() to complete a
> >> request is racy in case a timeout occurs while blk_mq_end_request() is
> >> in progress.
> >
> > Could you elaborate a bit more, blk_mq_end_request is exported function and
> > used by a lot of block drivers: scsi, dm, etc.
> > Is there an open bug report for the problem?
>
> Hi Jinpu,
>
> There is only one blk_mq_end_request() call in the SCSI code and it's
> inside the FC timeout handler (fc_bsg_job_timeout()). Calling
> blk_mq_end_request() from inside a timeout handler is fine but not to
> report to the block layer that a request has completed from outside the
> timeout handler after a request has started.
>
> The device mapper calls blk_mq_complete_request() to report request
> completion to the block layer. See also dm_complete_request().
> blk_mq_end_request() is only called by the device mapper from inside
> dm_softirq_done(). That last function is called from inside
> blk_mq_complete_request() and is not called directly.
>
> The NVMe PCIe driver only calls blk_mq_end_request() from inside
> nvme_complete_rq(). nvme_complete_rq() is called by the PCIe driver from
> inside nvme_pci_complete_rq() and that last function is called from
> inside blk_mq_complete_request().
>
> In other words, the SCSI core, the device mapper and the NVMe PCIe
> driver all use blk_mq_complete_request() to report request completion to
> the block layer from outside timeout handlers after a request has been
> started.
>
> This is not a new requirement. I think that the legacy block layer
> equivalent, blk_complete_request(), was introduced in 2006 and that
> since then block drivers are required to call blk_complete_request() to
> report completion of requests from outside a timeout handler after these
> have been started.
>
> Bart.
Thanks for the detailed explanation, I will switch to
blk_mq_complete_request(), will also drop the
softirq_done module parameter, not useful.
Regards,
Jinpu
