On Tue, May 22, 2018 at 10:37:32PM +0800, Ming Lei wrote: > On Tue, May 22, 2018 at 10:20 PM, Keith Busch > <keith.busch@xxxxxxxxxxxxxxx> wrote: > > In the event the driver requests a normal completion, the timeout work > > releasing the last reference doesn't do a second completion: it only > > The reference only covers request's lifetime, not related with completion. > > It isn't the last reference. When driver returns EH_HANDLED, blk-mq > will complete this request on extra time. > > Yes, if driver's timeout code and normal completion code can sync > about this completion, that should be fine, but the current behaviour > doesn't depend driver's sync since the req is always completed atomically > via the following way: > > 1) timeout > > if (mark_completed(rq)) > timed_out(rq) > > 2) normal completion > if (mark_completed(rq)) > complete(rq) > > For example, before nvme_timeout() is trying to run nvme_dev_disable(), > irq comes and this req is completed from normal completion path, but > nvme_timeout() still returns EH_HANDLED, and blk-mq may complete > the req one extra time since the normal completion path may not update > req's state yet. nvme_dev_disable tears down irq's, meaing their handling is already sync'ed before nvme_dev_disable can proceed. Whether the completion comes through nvme_irq, or through nvme_dev_disable, there is no way possible for nvme's timeout to return EH_HANDLED if the state was not updated prior to returning that status.