All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "peterz@infradead.org" <peterz@infradead.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"kernel-team@fb.com" <kernel-team@fb.com>,
	"oleg@redhat.com" <oleg@redhat.com>, "hch@lst.de" <hch@lst.de>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"jianchao.w.wang@oracle.com" <jianchao.w.wang@oracle.com>,
	"osandov@fb.com" <osandov@fb.com>,
	"tj@kernel.org" <tj@kernel.org>
Subject: Re: [PATCH 2/6] blk-mq: replace timeout synchronization with a RCU and generation based scheme
Date: Thu, 14 Dec 2017 21:42:48 +0000	[thread overview]
Message-ID: <1513287766.2475.73.camel@wdc.com> (raw)
In-Reply-To: <20171214202042.GG3326@worktop>

T24gVGh1LCAyMDE3LTEyLTE0IGF0IDIxOjIwICswMTAwLCBQZXRlciBaaWpsc3RyYSB3cm90ZToN
Cj4gT24gVGh1LCBEZWMgMTQsIDIwMTcgYXQgMDY6NTE6MTFQTSArMDAwMCwgQmFydCBWYW4gQXNz
Y2hlIHdyb3RlOg0KPiA+IE9uIFR1ZSwgMjAxNy0xMi0xMiBhdCAxMTowMSAtMDgwMCwgVGVqdW4g
SGVvIHdyb3RlOg0KPiA+ID4gKwl3cml0ZV9zZXFjb3VudF9iZWdpbigmcnEtPmdzdGF0ZV9zZXEp
Ow0KPiA+ID4gKwlibGtfbXFfcnFfdXBkYXRlX3N0YXRlKHJxLCBNUV9SUV9JTl9GTElHSFQpOw0K
PiA+ID4gKwlibGtfYWRkX3RpbWVyKHJxKTsNCj4gPiA+ICsJd3JpdGVfc2VxY291bnRfZW5kKCZy
cS0+Z3N0YXRlX3NlcSk7DQo+ID4gDQo+ID4gTXkgdW5kZXJzdGFuZGluZyBpcyB0aGF0IGJvdGgg
d3JpdGVfc2VxY291bnRfYmVnaW4oKSBhbmQgd3JpdGVfc2VxY291bnRfZW5kKCkNCj4gPiB0cmln
Z2VyIGEgd3JpdGUgbWVtb3J5IGJhcnJpZXIuIElzIGEgc2VxY291bnQgcmVhbGx5IGZhc3RlciB0
aGFuIGEgc3BpbmxvY2s/DQo+IA0KPiBZZXMgbG90cywgbm8gYXRvbWljIG9wZXJhdGlvbnMgYW5k
IG5vIHdhaXRpbmcuDQo+IA0KPiBUaGUgb25seSBjb25zdHJhaW50IGZvciB3cml0ZV9zZXFsb2Nr
IGlzIHRoYXQgdGhlcmUgbXVzdCBub3QgYmUgYW55DQo+IGNvbmN1cnJlbmN5Lg0KPiANCj4gQnV0
IG5vdyB0aGF0IEkgbG9vayBhdCB0aGlzIGFnYWluLCBUSiwgd2h5IGNhbid0IHRoZSBiZWxvdyBo
YXBwZW4/DQo+IA0KPiAJd3JpdGVfc2VxbG9ja19iZWdpbigpOw0KPiAJYmxrX21xX3JxX3VwZGF0
ZV9zdGF0ZShycSwgSU5fRkxJR0hUKTsNCj4gCWJsa19hZGRfdGltZXIocnEpOw0KPiAJPHRpbWVy
LWlycT4NCj4gCQlyZWFkX3NlcWNvdW50X2JlZ2luKCkNCj4gCQkJd2hpbGUgKHNlcSAmIDEpDQo+
IAkJCQljcHVyZWxheCgpOw0KPiAJCS8vIGxpZmUtbG9jaw0KPiAJPC90aW1lci1pcnE+DQo+IAl3
cml0ZV9zZXFsb2NrX2VuZCgpOw0KDQpIZWxsbyBQZXRlciwNCg0KU29tZSB0aW1lIGFnbyB0aGUg
YmxvY2sgbGF5ZXIgd2FzIGNoYW5nZWQgdG8gaGFuZGxlIHRpbWVvdXRzIGluIHRocmVhZCBjb250
ZXh0DQppbnN0ZWFkIG9mIGludGVycnVwdCBjb250ZXh0LiBTZWUgYWxzbyBjb21taXQgMjg3OTIy
ZWIwYjE4ICgiYmxvY2s6IGRlZmVyDQp0aW1lb3V0cyB0byBhIHdvcmtxdWV1ZSIpLg0KDQpCYXJ0
Lg==

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "peterz@infradead.org" <peterz@infradead.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"kernel-team@fb.com" <kernel-team@fb.com>,
	"oleg@redhat.com" <oleg@redhat.com>, "hch@lst.de" <hch@lst.de>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"jianchao.w.wang@oracle.com" <jianchao.w.wang@oracle.com>,
	"osandov@fb.com" <osandov@fb.com>,
	"tj@kernel.org" <tj@kernel.org>
Subject: Re: [PATCH 2/6] blk-mq: replace timeout synchronization with a RCU and generation based scheme
Date: Thu, 14 Dec 2017 21:42:48 +0000	[thread overview]
Message-ID: <1513287766.2475.73.camel@wdc.com> (raw)
In-Reply-To: <20171214202042.GG3326@worktop>

On Thu, 2017-12-14 at 21:20 +0100, Peter Zijlstra wrote:
> On Thu, Dec 14, 2017 at 06:51:11PM +0000, Bart Van Assche wrote:
> > On Tue, 2017-12-12 at 11:01 -0800, Tejun Heo wrote:
> > > +	write_seqcount_begin(&rq->gstate_seq);
> > > +	blk_mq_rq_update_state(rq, MQ_RQ_IN_FLIGHT);
> > > +	blk_add_timer(rq);
> > > +	write_seqcount_end(&rq->gstate_seq);
> > 
> > My understanding is that both write_seqcount_begin() and write_seqcount_end()
> > trigger a write memory barrier. Is a seqcount really faster than a spinlock?
> 
> Yes lots, no atomic operations and no waiting.
> 
> The only constraint for write_seqlock is that there must not be any
> concurrency.
> 
> But now that I look at this again, TJ, why can't the below happen?
> 
> 	write_seqlock_begin();
> 	blk_mq_rq_update_state(rq, IN_FLIGHT);
> 	blk_add_timer(rq);
> 	<timer-irq>
> 		read_seqcount_begin()
> 			while (seq & 1)
> 				cpurelax();
> 		// life-lock
> 	</timer-irq>
> 	write_seqlock_end();

Hello Peter,

Some time ago the block layer was changed to handle timeouts in thread context
instead of interrupt context. See also commit 287922eb0b18 ("block: defer
timeouts to a workqueue").

Bart.

  reply	other threads:[~2017-12-14 21:42 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-12 19:01 [PATCHSET v2] blk-mq: reimplement timeout handling Tejun Heo
2017-12-12 19:01 ` [PATCH 1/6] blk-mq: protect completion path with RCU Tejun Heo
2017-12-13  3:30   ` jianchao.wang
2017-12-13 16:13     ` Tejun Heo
2017-12-14  2:09       ` jianchao.wang
2017-12-14 17:01   ` Bart Van Assche
2017-12-14 17:01     ` Bart Van Assche
2017-12-14 18:14     ` tj
2017-12-12 19:01 ` [PATCH 2/6] blk-mq: replace timeout synchronization with a RCU and generation based scheme Tejun Heo
2017-12-12 21:37   ` Bart Van Assche
2017-12-12 21:37     ` Bart Van Assche
2017-12-12 21:44     ` tj
2017-12-13  5:07   ` jianchao.wang
2017-12-13 16:13     ` Tejun Heo
2017-12-14 18:51   ` Bart Van Assche
2017-12-14 18:51     ` Bart Van Assche
2017-12-14 19:19     ` tj
2017-12-14 21:13       ` Bart Van Assche
2017-12-14 21:13         ` Bart Van Assche
2017-12-15 13:30         ` tj
2017-12-14 20:20     ` Peter Zijlstra
2017-12-14 21:42       ` Bart Van Assche [this message]
2017-12-14 21:42         ` Bart Van Assche
2017-12-14 21:54         ` Peter Zijlstra
2017-12-15  2:12           ` jianchao.wang
2017-12-15  7:31             ` Peter Zijlstra
2017-12-15 15:14               ` jianchao.wang
2017-12-15  2:39           ` Mike Galbraith
2017-12-15  2:39             ` Mike Galbraith
2017-12-15 13:50       ` tj
2017-12-12 19:01 ` [PATCH 3/6] blk-mq: use blk_mq_rq_state() instead of testing REQ_ATOM_COMPLETE Tejun Heo
2017-12-12 19:01 ` [PATCH 4/6] blk-mq: make blk_abort_request() trigger timeout path Tejun Heo
2017-12-14 18:56   ` Bart Van Assche
2017-12-14 18:56     ` Bart Van Assche
2017-12-14 19:26     ` tj
2017-12-12 19:01 ` [PATCH 5/6] blk-mq: remove REQ_ATOM_COMPLETE usages from blk-mq Tejun Heo
2017-12-12 19:01 ` [PATCH 6/6] blk-mq: remove REQ_ATOM_STARTED Tejun Heo
2017-12-12 22:20   ` Bart Van Assche
2017-12-12 22:20     ` Bart Van Assche
2017-12-12 22:22     ` tj
2017-12-12 20:23 ` [PATCHSET v2] blk-mq: reimplement timeout handling Jens Axboe
2017-12-12 21:40   ` Tejun Heo
2017-12-20 23:41 ` Bart Van Assche
2017-12-20 23:41   ` Bart Van Assche
2017-12-21  0:08   ` tj
2017-12-21  1:00     ` Bart Van Assche
2017-12-21  1:00       ` Bart Van Assche
  -- strict thread matches above, loose matches on Subject: below --
2017-12-09 19:25 [PATCHSET] " Tejun Heo
2017-12-09 19:25 ` [PATCH 2/6] blk-mq: replace timeout synchronization with a RCU and generation based scheme Tejun Heo
2017-12-12 10:09   ` Peter Zijlstra
2017-12-12 18:02     ` Tejun Heo
2017-12-12 10:10   ` Peter Zijlstra
2017-12-12 18:03     ` Tejun Heo
2017-12-12 11:56   ` Peter Zijlstra
2017-12-12 18:04     ` Tejun Heo

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=1513287766.2475.73.camel@wdc.com \
    --to=bart.vanassche@wdc.com \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=jianchao.w.wang@oracle.com \
    --cc=kernel-team@fb.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=osandov@fb.com \
    --cc=peterz@infradead.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.