From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "hch@lst.de" <hch@lst.de>
Cc: "tj@kernel.org" <tj@kernel.org>,
"israelr@mellanox.com" <israelr@mellanox.com>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
"maxg@mellanox.com" <maxg@mellanox.com>,
"stable@vger.kernel.org" <stable@vger.kernel.org>,
"axboe@kernel.dk" <axboe@kernel.dk>,
"sagi@grimberg.me" <sagi@grimberg.me>
Subject: Re: [PATCH v4] blk-mq: Fix race conditions in request timeout handling
Date: Tue, 10 Apr 2018 13:26:39 +0000 [thread overview]
Message-ID: <f4bb15dff9dca995a9965204124caf2742798801.camel@wdc.com> (raw)
In-Reply-To: <20180410095537.GA19266@lst.de>
T24gVHVlLCAyMDE4LTA0LTEwIGF0IDExOjU1ICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90
ZToNCj4gSSBkb24ndCB0aGluayB3ZSBuZWVkIHRoZSBhdG9taWNfbG9uZ19jbXB4Y2hnLCBhbmQg
Y2FuIGRvIHdpdGggYSBwbGFpbg0KPiBjbXB4aGcuICBBbHNvIHVudGVybWluYXRlZCBjbXB4Y2hn
IGxvb3BzIGFyZSBhIGJhZCBpZGVhLCBidXQgSSB0aGluaw0KPiBib3RoIGNhbGxlcnMgYXJlIHBy
b3RlY3RlZCBmcm9tIG90aGVyIGNoYW5nZXMgc28gd2UgY2FuIHR1cm4gdGhhdA0KPiBpbnRvIGEg
V0FSTl9PTigpLg0KDQpIZWxsbyBDaHJpc3RvcGgsDQoNCkkgd2lsbCByZW1vdmUgdGhlIGJsa19t
cV9ycV91cGRhdGVfc3RhdGUoKSBmdW5jdGlvbiBzbyB0aGF0J3Mgb25lIHdoaWxlDQpsb29wIGxl
c3MuDQoNCkNhbiB5b3UgZXhwbGFpbiB3aHkgeW91IHRoaW5rIHRoYXQgdXNpbmcgY21weGNoZygp
IGlzIHNhZmUgaW4gdGhpcyBjb250ZXh0Pw0KVGhlIHJlZ3VsYXIgY29tcGxldGlvbiBwYXRoIGFu
ZCB0aGUgdGltZW91dCBjb2RlIGNhbiBib3RoIGV4ZWN1dGUgZS5nLiB0aGUNCmZvbGxvd2luZyBj
b2RlIGNvbmN1cnJlbnRseToNCg0KCWJsa19tcV9jaGFuZ2VfcnFfc3RhdGUocnEsIE1RX1JRX0lO
X0ZMSUdIVCwgTVFfUlFfQ09NUExFVEUpOw0KDQpUaGF0J3Mgd2h5IEkgdGhpbmsgdGhhdCB3ZSBu
ZWVkIGFuIGF0b21pYyBjb21wYXJlIGFuZCBleGNoYW5nZSBpbnN0ZWFkIG9mDQpjbXB4Y2hnKCku
IFdoYXQgSSBmb3VuZCBpbiB0aGUgSW50ZWwgU29mdHdhcmUgRGV2ZWxvcGVyIE1hbnVhbCBzZWVt
cyB0bw0KY29uZmlybSB0aGF0Og0KDQpEZXNjcmlwdGlvbg0KDQpDb21wYXJlcyB0aGUgdmFsdWUg
aW4gdGhlIEFMLCBBWCwgRUFYLCBvciBSQVggcmVnaXN0ZXIgd2l0aCB0aGUgZmlyc3QNCm9wZXJh
bmQgKGRlc3RpbmF0aW9uIG9wZXJhbmQpLiBJZiB0aGUgdHdvIHZhbHVlcyBhcmUgZXF1YWwsIHRo
ZSBzZWNvbmQNCm9wZXJhbmQgKHNvdXJjZSBvcGVyYW5kKSBpcyBsb2FkZWQgaW50byB0aGUgZGVz
dGluYXRpb24gb3BlcmFuZC4gT3RoZXJ3aXNlLA0KdGhlIGRlc3RpbmF0aW9uIG9wZXJhbmQgaXMg
bG9hZGVkIGludG8gdGhlIEFMLCBBWCwgRUFYIG9yIFJBWCByZWdpc3Rlci4gUkFYDQpyZWdpc3Rl
ciBpcyBhdmFpbGFibGUgb25seSBpbiA2NC1iaXQgbW9kZS4gVGhpcyBpbnN0cnVjdGlvbiBjYW4g
YmUgdXNlZA0Kd2l0aCBhIExPQ0sgcHJlZml4IHRvIGFsbG93IHRoZSBpbnN0cnVjdGlvbiB0byBi
ZSBleGVjdXRlZCBhdG9taWNhbGx5LiBUbw0Kc2ltcGxpZnkgdGhlIGludGVyZmFjZSB0byB0aGUg
cHJvY2Vzc29y4oCZcyBidXMsIHRoZSBkZXN0aW5hdGlvbiBvcGVyYW5kDQpyZWNlaXZlcyBhIHdy
aXRlIGN5Y2xlIHdpdGhvdXQgcmVnYXJkIHRvIHRoZSByZXN1bHQgb2YgdGhlIGNvbXBhcmlzb24u
IFRoZQ0KZGVzdGluYXRpb24gb3BlcmFuZCBpcyB3cml0dGVuIGJhY2sgaWYgdGhlIGNvbXBhcmlz
b24gZmFpbHM7IG90aGVyd2lzZSwgdGhlDQpzb3VyY2Ugb3BlcmFuZCBpcyB3cml0dGVuIGludG8g
dGhlIGRlc3RpbmF0aW9uLiAoVGhlIHByb2Nlc3NvciBuZXZlcg0KcHJvZHVjZXMgYSBsb2NrZWQg
cmVhZCB3aXRob3V0IGFsc28gcHJvZHVjaW5nIGEgbG9ja2VkIHdyaXRlLikNCg0KVGhhbmtzLA0K
DQpCYXJ0Lg0KDQoNCg0K
WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "hch@lst.de" <hch@lst.de>
Cc: "tj@kernel.org" <tj@kernel.org>,
"israelr@mellanox.com" <israelr@mellanox.com>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
"maxg@mellanox.com" <maxg@mellanox.com>,
"stable@vger.kernel.org" <stable@vger.kernel.org>,
"axboe@kernel.dk" <axboe@kernel.dk>,
"sagi@grimberg.me" <sagi@grimberg.me>
Subject: Re: [PATCH v4] blk-mq: Fix race conditions in request timeout handling
Date: Tue, 10 Apr 2018 13:26:39 +0000 [thread overview]
Message-ID: <f4bb15dff9dca995a9965204124caf2742798801.camel@wdc.com> (raw)
In-Reply-To: <20180410095537.GA19266@lst.de>
On Tue, 2018-04-10 at 11:55 +0200, Christoph Hellwig wrote:
> I don't think we need the atomic_long_cmpxchg, and can do with a plain
> cmpxhg. Also unterminated cmpxchg loops are a bad idea, but I think
> both callers are protected from other changes so we can turn that
> into a WARN_ON().
Hello Christoph,
I will remove the blk_mq_rq_update_state() function so that's one while
loop less.
Can you explain why you think that using cmpxchg() is safe in this context?
The regular completion path and the timeout code can both execute e.g. the
following code concurrently:
blk_mq_change_rq_state(rq, MQ_RQ_IN_FLIGHT, MQ_RQ_COMPLETE);
That's why I think that we need an atomic compare and exchange instead of
cmpxchg(). What I found in the Intel Software Developer Manual seems to
confirm that:
Description
Compares the value in the AL, AX, EAX, or RAX register with the first
operand (destination operand). If the two values are equal, the second
operand (source operand) is loaded into the destination operand. Otherwise,
the destination operand is loaded into the AL, AX, EAX or RAX register. RAX
register is available only in 64-bit mode. This instruction can be used
with a LOCK prefix to allow the instruction to be executed atomically. To
simplify the interface to the processor’s bus, the destination operand
receives a write cycle without regard to the result of the comparison. The
destination operand is written back if the comparison fails; otherwise, the
source operand is written into the destination. (The processor never
produces a locked read without also producing a locked write.)
Thanks,
Bart.
next prev parent reply other threads:[~2018-04-10 13:26 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
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 [this message]
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=f4bb15dff9dca995a9965204124caf2742798801.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=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.