All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "tj@kernel.org" <tj@kernel.org>, "axboe@kernel.dk" <axboe@kernel.dk>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"peterz@infradead.org" <peterz@infradead.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>,
	"jianchao.w.wang@oracle.com" <jianchao.w.wang@oracle.com>,
	"osandov@fb.com" <osandov@fb.com>
Subject: Re: [PATCH 2/6] blk-mq: replace timeout synchronization with a RCU and generation based scheme
Date: Tue, 12 Dec 2017 21:37:11 +0000	[thread overview]
Message-ID: <1513114630.2999.48.camel@wdc.com> (raw)
In-Reply-To: <20171212190134.535941-3-tj@kernel.org>

T24gVHVlLCAyMDE3LTEyLTEyIGF0IDExOjAxIC0wODAwLCBUZWp1biBIZW8gd3JvdGU6DQo+ICsv
Kg0KPiArICogQml0cyBmb3IgcmVxdWVzdC0+Z3N0YXRlLiAgVGhlIGxvd2VyIHR3byBiaXRzIGNh
cnJ5IE1RX1JRXyogc3RhdGUgdmFsdWUNCj4gKyAqIGFuZCB0aGUgdXBwZXIgYml0cyB0aGUgZ2Vu
ZXJhdGlvbiBudW1iZXIuDQo+ICsgKi8NCj4gK2VudW0gbXFfcnFfc3RhdGUgew0KPiArCU1RX1JR
X0lETEUJCT0gMCwNCj4gKwlNUV9SUV9JTl9GTElHSFQJCT0gMSwNCj4gKw0KPiArCU1RX1JRX1NU
QVRFX0JJVFMJPSAyLA0KPiArCU1RX1JRX1NUQVRFX01BU0sJPSAoMSA8PCBNUV9SUV9TVEFURV9C
SVRTKSAtIDEsDQo+ICsJTVFfUlFfR0VOX0lOQwkJPSAxIDw8IE1RX1JRX1NUQVRFX0JJVFMsDQo+
ICt9Ow0KPiArDQo+IEBAIC04NSw2ICs5OCwzOCBAQCBleHRlcm4gdm9pZCBibGtfbXFfcnFfdGlt
ZWRfb3V0KHN0cnVjdCByZXF1ZXN0ICpyZXEsIGJvb2wgcmVzZXJ2ZWQpOw0KPiArLyoqDQo+ICsg
KiBibGtfbXFfcnFfc3RhdGUoKSAtIHJlYWQgdGhlIGN1cnJlbnQgTVFfUlFfKiBzdGF0ZSBvZiBh
IHJlcXVlc3QNCj4gKyAqIEBycTogdGFyZ2V0IHJlcXVlc3QuDQo+ICsgKi8NCj4gK3N0YXRpYyBp
bmxpbmUgaW50IGJsa19tcV9ycV9zdGF0ZShzdHJ1Y3QgcmVxdWVzdCAqcnEpDQo+ICt7DQo+ICsJ
cmV0dXJuIFJFQURfT05DRShycS0+Z3N0YXRlKSAmIE1RX1JRX1NUQVRFX01BU0s7DQo+ICt9DQo+
ICsNCj4gKy8qKg0KPiArICogYmxrX21xX3JxX3VwZGF0ZV9zdGF0ZSgpIC0gc2V0IHRoZSBjdXJy
ZW50IE1RX1JRXyogc3RhdGUgb2YgYSByZXF1ZXN0DQo+ICsgKiBAcnE6IHRhcmdldCByZXF1ZXN0
Lg0KPiArICogQHN0YXRlOiBuZXcgc3RhdGUgdG8gc2V0Lg0KPiArICoNCj4gKyAqIFNldCBAcnEn
cyBzdGF0ZSB0byBAc3RhdGUuICBUaGUgY2FsbGVyIGlzIHJlc3BvbnNpYmxlIGZvciBlbnN1cmlu
ZyB0aGF0DQo+ICsgKiB0aGVyZSBhcmUgbm8gb3RoZXIgdXBkYXRlcnMuICBBIHJlcXVlc3QgY2Fu
IHRyYW5zaXRpb24gaW50byBJTl9GTElHSFQNCj4gKyAqIG9ubHkgZnJvbSBJRExFIGFuZCBkb2lu
ZyBzbyBpbmNyZW1lbnRzIHRoZSBnZW5lcmF0aW9uIG51bWJlci4NCj4gKyAqLw0KPiArc3RhdGlj
IGlubGluZSB2b2lkIGJsa19tcV9ycV91cGRhdGVfc3RhdGUoc3RydWN0IHJlcXVlc3QgKnJxLA0K
PiArCQkJCQkgIGVudW0gbXFfcnFfc3RhdGUgc3RhdGUpDQo+ICt7DQo+ICsJdTY0IG5ld192YWwg
PSAoUkVBRF9PTkNFKHJxLT5nc3RhdGUpICYgfk1RX1JRX1NUQVRFX01BU0spIHwgc3RhdGU7DQo+
ICsNCj4gKwlpZiAoc3RhdGUgPT0gTVFfUlFfSU5fRkxJR0hUKSB7DQo+ICsJCVdBUk5fT05fT05D
RShibGtfbXFfcnFfc3RhdGUocnEpICE9IE1RX1JRX0lETEUpOw0KPiArCQluZXdfdmFsICs9IE1R
X1JRX0dFTl9JTkM7DQo+ICsJfQ0KPiArDQo+ICsJLyogYXZvaWQgZXhwb3NpbmcgaW50ZXJpbSB2
YWx1ZXMgKi8NCj4gKwlXUklURV9PTkNFKHJxLT5nc3RhdGUsIG5ld192YWwpOw0KPiArfQ0KDQpI
ZWxsbyBUZWp1biwNCg0KSGF2ZSB5b3UgY29uc2lkZXJlZCB0aGUgZm9sbG93aW5nIGluc3RlYWQg
b2YgaW50cm9kdWNpbmcgTVFfUlFfSURMRSBhbmQNCk1RX1JRX0lOX0ZMSUdIVD8gSSB0aGluayB0
aGlzIGNvdWxkIGhlbHAgdG8gbGltaXQgdGhlIG51bWJlciBvZiBuZXcgYXRvbWljDQpvcGVyYXRp
b25zIGludHJvZHVjZWQgaW4gdGhlIGhvdCBwYXRoIGJ5IHRoaXMgcGF0Y2ggc2VyaWVzLg0KDQpz
dGF0aWMgaW5saW5lIGJvb2wgYmxrX21xX3JxX2luX2ZsaWdodChzdHJ1Y3QgcmVxdWVzdCAqcnEp
DQp7DQoJcmV0dXJuIGxpc3RfZW1wdHkoJnJxLT5xdWV1ZWxpc3QpOw0KfQ0KDQpUaGFua3MsDQoN
CkJhcnQu

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "tj@kernel.org" <tj@kernel.org>, "axboe@kernel.dk" <axboe@kernel.dk>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"peterz@infradead.org" <peterz@infradead.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>,
	"jianchao.w.wang@oracle.com" <jianchao.w.wang@oracle.com>,
	"osandov@fb.com" <osandov@fb.com>
Subject: Re: [PATCH 2/6] blk-mq: replace timeout synchronization with a RCU and generation based scheme
Date: Tue, 12 Dec 2017 21:37:11 +0000	[thread overview]
Message-ID: <1513114630.2999.48.camel@wdc.com> (raw)
In-Reply-To: <20171212190134.535941-3-tj@kernel.org>

On Tue, 2017-12-12 at 11:01 -0800, Tejun Heo wrote:
> +/*
> + * Bits for request->gstate.  The lower two bits carry MQ_RQ_* state value
> + * and the upper bits the generation number.
> + */
> +enum mq_rq_state {
> +	MQ_RQ_IDLE		= 0,
> +	MQ_RQ_IN_FLIGHT		= 1,
> +
> +	MQ_RQ_STATE_BITS	= 2,
> +	MQ_RQ_STATE_MASK	= (1 << MQ_RQ_STATE_BITS) - 1,
> +	MQ_RQ_GEN_INC		= 1 << MQ_RQ_STATE_BITS,
> +};
> +
> @@ -85,6 +98,38 @@ extern void blk_mq_rq_timed_out(struct request *req, bool reserved);
> +/**
> + * blk_mq_rq_state() - read the current MQ_RQ_* state of a request
> + * @rq: target request.
> + */
> +static inline int blk_mq_rq_state(struct request *rq)
> +{
> +	return READ_ONCE(rq->gstate) & MQ_RQ_STATE_MASK;
> +}
> +
> +/**
> + * blk_mq_rq_update_state() - set the current MQ_RQ_* state of a request
> + * @rq: target request.
> + * @state: new state to set.
> + *
> + * Set @rq's state to @state.  The caller is responsible for ensuring that
> + * there are no other updaters.  A request can transition into IN_FLIGHT
> + * only from IDLE and doing so increments the generation number.
> + */
> +static inline void blk_mq_rq_update_state(struct request *rq,
> +					  enum mq_rq_state state)
> +{
> +	u64 new_val = (READ_ONCE(rq->gstate) & ~MQ_RQ_STATE_MASK) | state;
> +
> +	if (state == MQ_RQ_IN_FLIGHT) {
> +		WARN_ON_ONCE(blk_mq_rq_state(rq) != MQ_RQ_IDLE);
> +		new_val += MQ_RQ_GEN_INC;
> +	}
> +
> +	/* avoid exposing interim values */
> +	WRITE_ONCE(rq->gstate, new_val);
> +}

Hello Tejun,

Have you considered the following instead of introducing MQ_RQ_IDLE and
MQ_RQ_IN_FLIGHT? I think this could help to limit the number of new atomic
operations introduced in the hot path by this patch series.

static inline bool blk_mq_rq_in_flight(struct request *rq)
{
	return list_empty(&rq->queuelist);
}

Thanks,

Bart.

  reply	other threads:[~2017-12-12 21:37 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 [this message]
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
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=1513114630.2999.48.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.