All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "ming.lei@redhat.com" <ming.lei@redhat.com>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"israelr@mellanox.com" <israelr@mellanox.com>,
	"sagi@grimberg.me" <sagi@grimberg.me>, "hch@lst.de" <hch@lst.de>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"maxg@mellanox.com" <maxg@mellanox.com>,
	"tj@kernel.org" <tj@kernel.org>
Subject: Re: [PATCH v4] blk-mq: Fix race conditions in request timeout handling
Date: Tue, 10 Apr 2018 14:09:33 +0000	[thread overview]
Message-ID: <7c4a8b019182d5a9259ef20e6462b3f6a533abed.camel@wdc.com> (raw)
In-Reply-To: <20180410135541.GA22340@ming.t460p>

T24gVHVlLCAyMDE4LTA0LTEwIGF0IDIxOjU1ICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gVGhl
biBJIGhhdmUgc2FtZSBxdWVzdGlvbiB3aXRoIEppYW5jaGFvLCB3aGF0IGlzIHRoZSBhY3R1YWwg
ZG91YmxlDQo+IGNvbXBsZXRlIGluIGxpbnVzIHRyZWUgYmV0d2VlbiBCTEtfRUhfUkVTRVRfVElN
RVIgYW5kIG5vcm1hbCBjb21wbGV0aW9uPw0KPiANCj4gRm9sbG93cyBteSB1bmRlcnN0YW5kaW5n
Og0KPiANCj4gMSkgd2hlbiB0aW1lb3V0IGlzIGRldGVjdGVkIG9uIG9uZSByZXF1ZXN0LCBpdHMg
YWJvcnRlZF9nc3RhdGUgaXMNCj4gdXBkYXRlZCBpbiBibGtfbXFfY2hlY2tfZXhwaXJlZCgpLg0K
PiANCj4gMikgcnVuIHN5bmNocm9uaXplX3JjdSgpLCBhbmQgbWFrZSBzdXJlIGFsbCBwZW5kaW5n
IGNvbXBsZXRpb24gaXMgZG9uZQ0KPiANCj4gMykgcnVuIGJsa19tcV9ycV90aW1lZF9vdXQoKQ0K
PiAtIHJldCA9IG9wcy0+dGltZW91dA0KPiAtIGJsa19tcV9ycV91cGRhdGVfYWJvcnRlZF9nc3Rh
dGUocmVxLCAwKQ0KPiAtIGJsa19hZGRfdGltZXIocmVxKTsNCj4gDQo+IElmIG5vcm1hbCBjb21w
bGV0aW9uIGlzIGRvbmUgYmV0d2VlbiAxKSBhbmQgcmVzZXQgYWJvcnRlZF9nc3RhdGUgaW4gMyks
DQo+IGJsa19tcV9jb21wbGV0ZV9yZXF1ZXN0KCkgd2lsbCBiZSBjYWxsZWQsIGFuZCBmb3VuZCB0
aGF0IGFib3J0ZWRfZ3N0YXRlDQo+IGlzIHNldCwgdGhlbiB0aGUgcnEgd29uJ3QgYmUgY29tcGxl
dGVkIHJlYWxseS4NCj4gDQo+IElmIG5vcm1hbCBjb21wbGV0aW9uIGlzIGRvbmUgYWZ0ZXIgcmVz
ZXQgYWJvcnRlZF9nc3RhdGUgaW4gMyksIGl0IHNob3VsZA0KPiBiZSBzYW1lIHdpdGggYXBwbHlp
bmcgdGhpcyBwYXRjaC4NCg0KSGVsbG8gTWluZywNCg0KUGxlYXNlIGtlZXAgaW4gbWluZCB0aGF0
IGFsbCBzeW5jaHJvbml6ZV9yY3UoKSBkb2VzIGlzIHRvIHdhaXQgZm9yIHByZS0NCmV4aXN0aW5n
IFJDVSByZWFkZXJzIHRvIGZpbmlzaC4gc3luY2hyb25pemVfcmN1KCkgZG9lcyBub3QgcHJldmVu
dCB0aGF0IG5ldw0KcmN1X3JlYWRfbG9jaygpIGNhbGxzIGhhcHBlbi4gSXQgaXMgZS5nLiBwb3Nz
aWJsZSB0aGF0IGFmdGVyDQpibGtfbXFfcnFfdXBkYXRlX2Fib3J0ZWRfZ3N0YXRlKHJlcSwgMCkg
aGFzIGJlZW4gZXhlY3V0ZWQgdGhhdCBhIHJlZ3VsYXINCmNvbXBsZXRpb24gb2NjdXJzLiBJZiB0
aGF0IHJlcXVlc3QgaXMgbm90IHJldXNlZCBiZWZvcmUgdGhlIHRpbWVyIHRoYXQgd2FzDQpyZXN0
YXJ0ZWQgYnkgdGhlIHRpbWVvdXQgY29kZSBleHBpcmVzLCB0aGF0IHJlcXVlc3Qgd2lsbCBiZSBj
b21wbGV0ZWQgdHdpY2UuDQoNCkJhcnQuDQoNCg0KDQo=

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "ming.lei@redhat.com" <ming.lei@redhat.com>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"israelr@mellanox.com" <israelr@mellanox.com>,
	"sagi@grimberg.me" <sagi@grimberg.me>, "hch@lst.de" <hch@lst.de>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"maxg@mellanox.com" <maxg@mellanox.com>,
	"tj@kernel.org" <tj@kernel.org>
Subject: Re: [PATCH v4] blk-mq: Fix race conditions in request timeout handling
Date: Tue, 10 Apr 2018 14:09:33 +0000	[thread overview]
Message-ID: <7c4a8b019182d5a9259ef20e6462b3f6a533abed.camel@wdc.com> (raw)
In-Reply-To: <20180410135541.GA22340@ming.t460p>

On Tue, 2018-04-10 at 21:55 +0800, Ming Lei wrote:
> Then I have same question with Jianchao, what is the actual double
> complete in linus tree between BLK_EH_RESET_TIMER and normal completion?
> 
> Follows my understanding:
> 
> 1) when timeout is detected on one request, its aborted_gstate is
> updated in blk_mq_check_expired().
> 
> 2) run synchronize_rcu(), and make sure all pending completion is done
> 
> 3) run blk_mq_rq_timed_out()
> - ret = ops->timeout
> - blk_mq_rq_update_aborted_gstate(req, 0)
> - blk_add_timer(req);
> 
> If normal completion is done between 1) and reset aborted_gstate in 3),
> blk_mq_complete_request() will be called, and found that aborted_gstate
> is set, then the rq won't be completed really.
> 
> If normal completion is done after reset aborted_gstate in 3), it should
> be same with applying this patch.

Hello Ming,

Please keep in mind that all synchronize_rcu() does is to wait for pre-
existing RCU readers to finish. synchronize_rcu() does not prevent that new
rcu_read_lock() calls happen. It is e.g. possible that after
blk_mq_rq_update_aborted_gstate(req, 0) has been executed that a regular
completion occurs. If that request is not reused before the timer that was
restarted by the timeout code expires, that request will be completed twice.

Bart.




  reply	other threads:[~2018-04-10 14:09 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-10  1:34 [PATCH v4] blk-mq: Fix race conditions in request timeout handling Bart Van Assche
2018-04-10  7:59 ` jianchao.wang
2018-04-10 10:04   ` Ming Lei
2018-04-10 12:04     ` Shan Hai
2018-04-10 13:01   ` Bart Van Assche
2018-04-10 13:01     ` Bart Van Assche
2018-04-10 14:32     ` jianchao.wang
2018-04-10  8:41 ` Ming Lei
2018-04-10 12:58   ` Bart Van Assche
2018-04-10 12:58     ` Bart Van Assche
2018-04-10 13:55     ` Ming Lei
2018-04-10 14:09       ` Bart Van Assche [this message]
2018-04-10 14:09         ` Bart Van Assche
2018-04-10 14:30         ` Ming Lei
2018-04-10 15:02           ` Bart Van Assche
2018-04-10 15:02             ` Bart Van Assche
2018-04-10 15:25             ` Ming Lei
2018-04-10 15:30               ` tj
2018-04-10 15:38                 ` Ming Lei
2018-04-10 15:40                   ` tj
2018-04-10 21:33                     ` tj
2018-04-10 21:46                       ` Bart Van Assche
2018-04-10 21:46                         ` Bart Van Assche
2018-04-10 21:54                         ` tj
2018-04-11 12:50                           ` Bart Van Assche
2018-04-11 12:50                             ` Bart Van Assche
2018-04-11 14:16                             ` tj
2018-04-11 18:38                             ` Martin Steigerwald
2018-04-11 18:38                               ` Martin Steigerwald
2018-04-11 14:24                           ` Sagi Grimberg
2018-04-11 14:43                             ` tj
2018-04-11 16:16                             ` Israel Rukshin
2018-04-11 17:07                               ` tj
2018-04-11 21:31                                 ` tj
2018-04-12  8:59                                   ` Israel Rukshin
2018-04-12 13:35                                     ` tj
2018-04-15 12:28                                       ` Israel Rukshin
2018-04-18 16:34                           ` Bart Van Assche
2018-04-10  9:55 ` Christoph Hellwig
2018-04-10 13:26   ` Bart Van Assche
2018-04-10 13:26     ` Bart Van Assche
2018-04-10 14:50     ` hch
2018-04-10 14:41   ` Jens Axboe
2018-04-10 14:20 ` Tejun Heo
2018-04-10 14:30   ` Bart Van Assche
2018-04-10 14:30     ` Bart Van Assche
2018-04-10 14:33     ` tj

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=7c4a8b019182d5a9259ef20e6462b3f6a533abed.camel@wdc.com \
    --to=bart.vanassche@wdc.com \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=israelr@mellanox.com \
    --cc=linux-block@vger.kernel.org \
    --cc=maxg@mellanox.com \
    --cc=ming.lei@redhat.com \
    --cc=sagi@grimberg.me \
    --cc=stable@vger.kernel.org \
    --cc=tj@kernel.org \
    /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.