All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "hch@lst.de" <hch@lst.de>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"israelr@mellanox.com" <israelr@mellanox.com>,
	"sagi@grimberg.me" <sagi@grimberg.me>,
	"sebott@linux.ibm.com" <sebott@linux.ibm.com>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"ming.lei@redhat.com" <ming.lei@redhat.com>,
	"jianchao.w.wang@oracle.com" <jianchao.w.wang@oracle.com>,
	"maxg@mellanox.com" <maxg@mellanox.com>,
	"tj@kernel.org" <tj@kernel.org>
Subject: Re: [PATCH v9 2/2] blk-mq: Rework blk-mq timeout handling again
Date: Wed, 16 May 2018 16:47:54 +0000	[thread overview]
Message-ID: <bd17181940994eb459e84e4d5940b1af8a725ded.camel@wdc.com> (raw)
In-Reply-To: <20180516162432.GA4398@lst.de>

T24gV2VkLCAyMDE4LTA1LTE2IGF0IDE4OjI0ICswMjAwLCBoY2hAbHN0LmRlIHdyb3RlOg0KPiBP
biBXZWQsIE1heSAxNiwgMjAxOCBhdCAwNDoxNzo0MlBNICswMDAwLCBCYXJ0IFZhbiBBc3NjaGUg
d3JvdGU6DQo+ID4gVGhlcmUgaXMgYW5vdGhlciByZWFzb24gdGhlIGRlYWRsaW5lIGlzIGluY2x1
ZGVkIGluIHRoZSBhdG9taWMgb3BlcmF0aW9uLA0KPiA+IG5hbWVseSB0byBoYW5kbGUgcmFjZXMg
YmV0d2VlbiB0aGUgQkxLX0VIX1JFU0VUX1RJTUVSIGNhc2UgaW4gYmxrX21xX3JxX3RpbWVkX291
dCgpDQo+ID4gYW5kIGJsa19tcV9jb21wbGV0ZV9yZXF1ZXN0KCkuIEkgZG9uJ3QgdGhpbmsgdGhh
dCByYWNlIGlzIGFkZHJlc3NlZCBwcm9wZXJseSBieQ0KPiA+IHlvdXIgcGF0Y2guIEkgd2lsbCBz
ZWUgd2hhdCBJIGNhbiBkbyB0byBhZGRyZXNzIHRoYXQgcmFjZSB3aXRob3V0IHVzaW5nIDY0LWJp
dA0KPiA+IGF0b21pYyBvcGVyYXRpb25zLg0KPiANCj4gSSBtaWdodCBiZSBtaXNzaW5nIHNvbWV0
aGluZyBoZXJlLCBzbyBwbGVhc2UgaGVscCBtZSB1bmRlcnN0YW5kDQo+IHdoYXQgaXMgbWlzc2lu
Zy4NCj4gDQo+IElmIHdlIHJlc3RhcnQgdGhlIHRpbWVyIGluIGJsa19tcV9ycV90aW1lZF9vdXQg
d2UgYWxzbyBidW1wIHRoZQ0KPiBnZW5lcmF0aW9uIGF0IHRoZSBzYW1lIHRpbWUgYXMgd2UgcmVz
ZXQgdGhlIGRlYWRsaW5lIGluIHlvdXIgb2xkDQo+IHBhdGNoLiAgV2l0aCB0aGlzIHBhdGNoIHdl
IG9ubHkgYnVtcCB0aGUgZ2VuZXJhdGlvbiwgYnV0IHRoYXQgc2hvdWxkDQo+IGJlIGVub3VnaCB0
byBhZGRyZXNzIHRoZSByZXN0LCBvciBub3Q/DQoNCkhlbGxvIENocmlzdG9waCwNCg0KSSB0aGlu
ayB5b3VyIHBhdGNoIGNoYW5nZXMgdGhlIG9yZGVyIG9mIGNoYW5naW5nIHRoZSByZXF1ZXN0IHN0
YXRlIGFuZA0KY2FsbGluZyBtb2RfdGltZXIoKS4gSW4gbXkgcGF0Y2ggdGhlIHJlcXVlc3Qgc3Rh
dGUgYW5kIHRoZSBkZWFkbGluZSBhcmUNCnVwZGF0ZWQgZmlyc3QgYW5kIG1vZF90aW1lcigpIGlz
IGNhbGxlZCBhZnRlcndhcmRzLiBJIHRoaW5rIHlvdXIgcGF0Y2gNCmNoYW5nZXMgdGhlIG9yZGVy
IG9mIHRoZXNlIG9wZXJhdGlvbnMgaW50byB0aGUgZm9sbG93aW5nOg0KKDEpIF9fYmxrX21xX3N0
YXJ0X3JlcXVlc3QoKSBzZXRzIHRoZSByZXF1ZXN0IGRlYWRsaW5lLg0KKDIpIF9fYmxrX21xX3N0
YXJ0X3JlcXVlc3QoKSBjYWxscyBfX2Jsa19hZGRfdGltZXIoKSB3aGljaCBpbiB0dXJuIGNhbGxz
DQogICAgbW9kX3RpbWVyKCkuDQooMykgX19ibGtfbXFfc3RhcnRfcmVxdWVzdCgpIGNoYW5nZXMg
dGhlIHJlcXVlc3Qgc3RhdGUgaW50byBNUV9SUV9JTl9GTElHSFQuDQoNCkluIHRoZSB1bmxpa2Vs
eSBldmVudCBvZiBhIHNpZ25pZmljYW50IGRlbGF5IGJldHdlZW4gKDIpIGFuZCAoMykgaXQgY2Fu
DQpoYXBwZW4gdGhhdCB0aGUgdGltZXIgZmlyZXMgYW5kIGV4YW1pbmVzIGFuZCBpZ25vcmVzIHRo
ZSByZXF1ZXN0IGJlY2F1c2UNCml0cyBzdGF0ZSBkaWZmZXJzIGZyb20gTVFfUlFfSU5fRkxJR0hU
LiBJZiB0aGUgcmVxdWVzdCBmb3Igd2hpY2ggdGhpcw0KaGFwcGVuZWQgdGltZXMgb3V0IGl0cyB0
aW1lb3V0IHdpbGwgb25seSBiZSBoYW5kbGVkIHRoZSBuZXh0IHRpbWUNCmJsa19tcV90aW1lb3V0
X3dvcmsoKSBpcyBjYWxsZWQuIElzIHRoaXMgdGhlIGJlaGF2aW9yIHlvdSBpbnRlbmRlZD8NCg0K
VGhhbmtzLA0KDQpCYXJ0Lg0K

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "hch@lst.de" <hch@lst.de>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"israelr@mellanox.com" <israelr@mellanox.com>,
	"sagi@grimberg.me" <sagi@grimberg.me>,
	"sebott@linux.ibm.com" <sebott@linux.ibm.com>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"ming.lei@redhat.com" <ming.lei@redhat.com>,
	"jianchao.w.wang@oracle.com" <jianchao.w.wang@oracle.com>,
	"maxg@mellanox.com" <maxg@mellanox.com>,
	"tj@kernel.org" <tj@kernel.org>
Subject: Re: [PATCH v9 2/2] blk-mq: Rework blk-mq timeout handling again
Date: Wed, 16 May 2018 16:47:54 +0000	[thread overview]
Message-ID: <bd17181940994eb459e84e4d5940b1af8a725ded.camel@wdc.com> (raw)
In-Reply-To: <20180516162432.GA4398@lst.de>

On Wed, 2018-05-16 at 18:24 +0200, hch@lst.de wrote:
> On Wed, May 16, 2018 at 04:17:42PM +0000, Bart Van Assche wrote:
> > There is another reason the deadline is included in the atomic operation,
> > namely to handle races between the BLK_EH_RESET_TIMER case in blk_mq_rq_timed_out()
> > and blk_mq_complete_request(). I don't think that race is addressed properly by
> > your patch. I will see what I can do to address that race without using 64-bit
> > atomic operations.
> 
> I might be missing something here, so please help me understand
> what is missing.
> 
> If we restart the timer in blk_mq_rq_timed_out we also bump the
> generation at the same time as we reset the deadline in your old
> patch.  With this patch we only bump the generation, but that should
> be enough to address the rest, or not?

Hello Christoph,

I think your patch changes the order of changing the request state and
calling mod_timer(). In my patch the request state and the deadline are
updated first and mod_timer() is called afterwards. I think your patch
changes the order of these operations into the following:
(1) __blk_mq_start_request() sets the request deadline.
(2) __blk_mq_start_request() calls __blk_add_timer() which in turn calls
    mod_timer().
(3) __blk_mq_start_request() changes the request state into MQ_RQ_IN_FLIGHT.

In the unlikely event of a significant delay between (2) and (3) it can
happen that the timer fires and examines and ignores the request because
its state differs from MQ_RQ_IN_FLIGHT. If the request for which this
happened times out its timeout will only be handled the next time
blk_mq_timeout_work() is called. Is this the behavior you intended?

Thanks,

Bart.

  reply	other threads:[~2018-05-16 16:47 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-15 22:51 [PATCH v10 0/2] blk-mq: Rework blk-mq timeout handling again Bart Van Assche
2018-05-15 22:51 ` [PATCH v10 1/2] arch/*: Add CONFIG_ARCH_HAVE_CMPXCHG64 Bart Van Assche
2018-05-16  4:15   ` Michael Ellerman
2018-05-18 20:33   ` Palmer Dabbelt
2018-05-15 22:51 ` [PATCH v10 2/2] blk-mq: Rework blk-mq timeout handling again Bart Van Assche
2018-05-15 22:51 ` [PATCH v9 0/2] " Bart Van Assche
2018-05-15 22:51 ` [PATCH v9 1/2] arch/*: Add CONFIG_ARCH_HAVE_CMPXCHG64 Bart Van Assche
2018-05-16  6:07   ` Christoph Hellwig
2018-05-15 22:51 ` [PATCH v9 2/2] blk-mq: Rework blk-mq timeout handling again Bart Van Assche
2018-05-16 12:51   ` Christoph Hellwig
2018-05-16 16:17     ` Bart Van Assche
2018-05-16 16:17       ` Bart Van Assche
2018-05-16 16:24       ` hch
2018-05-16 16:47         ` Bart Van Assche [this message]
2018-05-16 16:47           ` Bart Van Assche
2018-05-16 17:31           ` hch
2018-05-16 18:06             ` Bart Van Assche
2018-05-16 18:06               ` Bart Van Assche
  -- strict thread matches above, loose matches on Subject: below --
2018-05-14 18:46 [PATCH v9 0/2] " Bart Van Assche
2018-05-14 18:46 ` [PATCH v9 2/2] " Bart Van Assche
2018-05-15 13:49   ` Sebastian Ott

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=bd17181940994eb459e84e4d5940b1af8a725ded.camel@wdc.com \
    --to=bart.vanassche@wdc.com \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=israelr@mellanox.com \
    --cc=jianchao.w.wang@oracle.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxg@mellanox.com \
    --cc=ming.lei@redhat.com \
    --cc=sagi@grimberg.me \
    --cc=sebott@linux.ibm.com \
    --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.