Re: [PATCH v2 7/7] [RFC] nvme: Fix a race condition
On 10/11/16 09:46, Christoph Hellwig wrote: On Wed, Sep 28, 2016 at 05:01:45PM -0700, Bart Van Assche wrote: Avoid that nvme_queue_rq() is still running when nvme_stop_queues() returns. Untested. Signed-off-by: Bart Van Assche Cc: Keith Busch Cc: Christoph Hellwig Cc: Sagi Grimberg --- drivers/nvme/host/core.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index d791fba..98f1f29 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -201,13 +201,9 @@ fail: void nvme_requeue_req(struct request *req) { - unsigned long flags; - blk_mq_requeue_request(req); - spin_lock_irqsave(req->q->queue_lock, flags); - if (!blk_mq_queue_stopped(req->q)) - blk_mq_kick_requeue_list(req->q); - spin_unlock_irqrestore(req->q->queue_lock, flags); + WARN_ON_ONCE(blk_mq_queue_stopped(req->q)); + blk_mq_kick_requeue_list(req->q); } EXPORT_SYMBOL_GPL(nvme_requeue_req); Can we just add a 'bool kick' argument to blk_mq_requeue_request and move all this handling to the core? Hello Christoph, That sounds like a good idea to me. Thanks also for the other review comments you posted on this patch series. I will rework patch 6/7 such that the code for waiting is moved into the SCSI core. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/7] [RFC] nvme: Fix a race condition
On Wed, Sep 28, 2016 at 05:01:45PM -0700, Bart Van Assche wrote: > Avoid that nvme_queue_rq() is still running when nvme_stop_queues() > returns. Untested. > > Signed-off-by: Bart Van Assche > Cc: Keith Busch > Cc: Christoph Hellwig > Cc: Sagi Grimberg > --- > drivers/nvme/host/core.c | 20 > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index d791fba..98f1f29 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -201,13 +201,9 @@ fail: > > void nvme_requeue_req(struct request *req) > { > - unsigned long flags; > - > blk_mq_requeue_request(req); > - spin_lock_irqsave(req->q->queue_lock, flags); > - if (!blk_mq_queue_stopped(req->q)) > - blk_mq_kick_requeue_list(req->q); > - spin_unlock_irqrestore(req->q->queue_lock, flags); > + WARN_ON_ONCE(blk_mq_queue_stopped(req->q)); > + blk_mq_kick_requeue_list(req->q); > } > EXPORT_SYMBOL_GPL(nvme_requeue_req); Can we just add a 'bool kick' argument to blk_mq_requeue_request and move all this handling to the core? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/7] [RFC] nvme: Fix a race condition
Avoid that nvme_queue_rq() is still running when nvme_stop_queues() returns. Untested. Signed-off-by: Bart Van Assche Cc: Keith Busch Cc: Christoph Hellwig Cc: Sagi Grimberg Bart this looks really good! and possibly fixes an issue I've been chasing with fabrics a while ago. I'll take it for testing but you can add my: Reviewed-by: Sagi Grimberg -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html