All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Ming Lei <ming.lei@canonical.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Ming Lei <ming.lei@canoical.com>
Subject: Re: [PATCH 2/2] blk-mq: remove unnecessary blk_clear_rq_complete()
Date: Thu, 18 Sep 2014 18:26:38 +0200	[thread overview]
Message-ID: <20140918162638.GA3950@lst.de> (raw)
In-Reply-To: <CACVXFVOsHO28yTQN8uPgOgtrTjrs52Le01wChxdYyjeBr0ZcOQ@mail.gmail.com>

On Thu, Sep 18, 2014 at 09:33:35AM +0800, Ming Lei wrote:
> > Why yo you think it's not nessecary?  The request is not in the drivers
> 
> The COMPLETED flag will be cleared in blk_mq_start_request(), so
> it needn't to be cleared here, and it is a bit early and might cause
> a tiny race window.
> 
> > hand at this point, so it should not be marked started.  Maybe I'm missing
> > something, but this sounds like it could very likely cause regressions.
> 
> The STARTED flag has been cleared in __blk_mq_requeue_request(),
> I mean the COMPLETED flag, consider the following situation:
> 
> - one req is queued to driver, and the req is completed from
> the device just before its timeout expired
> - the driver checks the result and finds it need to requeue for some reason
> - blk_mq_requeue_request() is called to requeue the req
> - COMPLETED is cleared before clearing STARTED because of
> writes reorder
> - timeout just comes between the two writes
> - then the request may be completed another time because of timeout

Ok, I guess you are right on this one.  Objection cleared.

      reply	other threads:[~2014-09-18 16:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-17  9:47 [PATCH 0/2] blk-mq: fix race between timeout and queue_rq Ming Lei
2014-09-17  9:47 ` [PATCH 1/2] " Ming Lei
2014-09-17 16:44   ` Christoph Hellwig
2014-09-17  9:47 ` [PATCH 2/2] blk-mq: remove unnecessary blk_clear_rq_complete() Ming Lei
2014-09-17 16:48   ` Christoph Hellwig
2014-09-18  1:33     ` Ming Lei
2014-09-18 16:26       ` Christoph Hellwig [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140918162638.GA3950@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@canoical.com \
    --cc=ming.lei@canonical.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.