From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
Damien Le Moal <Damien.LeMoal@wdc.com>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
"axboe@kernel.dk" <axboe@kernel.dk>
Cc: "hch@lst.de" <hch@lst.de>
Subject: Re: [PATCH V6 13/14] block: mq-deadline: Limit write request dispatch for zoned block devices
Date: Mon, 2 Oct 2017 23:44:07 +0000 [thread overview]
Message-ID: <1506987846.3368.69.camel@wdc.com> (raw)
In-Reply-To: <20171002071535.8007-14-damien.lemoal@wdc.com>
T24gTW9uLCAyMDE3LTEwLTAyIGF0IDE2OjE1ICswOTAwLCBEYW1pZW4gTGUgTW9hbCB3cm90ZToN
Cj4gV2hlbiBkaXNwYXRjaGluZyB3cml0ZSByZXF1ZXN0cyB0byBhIHpvbmVkIGJsb2NrIGRldmlj
ZSwgb25seSBhbGxvdw0KPiByZXF1ZXN0cyB0YXJnZXRpbmcgYW4gdW5sb2NrZWQgem9uZS4gUmVx
dWVzdHMgdGFyZ2V0aW5nIGEgbG9ja2VkIHpvbmUNCj4gYXJlIGxlZnQgaW4gdGhlIHNjaGVkdWxl
ciBxdWV1ZSB0byBwcmVzZXJ2ZSB0aGUgaW5pdGlhbCB3cml0ZSBvcmRlci4NCj4gSWYgbm8gd3Jp
dGUgcmVxdWVzdCBjYW4gYmUgZGlzcGF0Y2hlZCwgYWxsb3cgcmVhZHMgdG8gYmUgZGlzcGF0Y2hl
ZA0KPiBldmVuIGlmIHRoZSB3cml0ZSBiYXRjaCBpcyBub3QgZG9uZS4NCj4NCj4gVG8gZW5zdXJl
IHRoYXQgdGhlIHNlYXJjaCBmb3IgYW4gYXBwcm9wcmlhdGUgd3JpdGUgcmVxdWVzdCBpcyBhdG9t
aWMNCj4gaW4gZGVhZGxpbmVfZmlmb19yZXF1ZXN0KCkgYW5kIGRlYWRsaW5lX25leHRfcmVxdWVz
dCgpIHdpdGggcmVhZ3JkIHRvDQogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIF5eXl5eXg0KICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICByZWdhcmQ/DQo+IHdyaXRlIHJl
cXVlc3RzIHpvbmUgbG9jayBzdGF0ZSwgaW50cm9kdWNlIHRoZSBzcGlubG9jayB6b25lX2xvY2su
DQo+IEhvbGRpbmcgdGhpcyBsb2NrIHdoaWxlIGRvaW5nIHRoZSBzZWFyY2ggaW4gdGhlc2UgZnVu
Y3Rpb25zIGFzIHdlbGwgYXMNCj4gd2hlbiB1bmxvY2tpbmcgdGhlIHRhcmdldCB6b25lIG9mIGEg
Y29tcGxldGVkIHdyaXRlIHJlcXVlc3QgaW4NCj4gZGRfY29tcGxldGVkX3JlcXVlc3QoKSBlbnN1
cmUgdGhhdCB0aGUgc2VhcmNoIGRvZXMgbm90IHBpY2t1cCBhIHdyaXRlDQo+IHJlcXVlc3QgaW4g
dGhlIG1pZGRsZSBvZiBhIHpvbmUgcXVldWVkIHdyaXRlIHNlcXVlbmNlLg0KDQpTaW5jZSB0aGVy
ZSBpcyBhbHJlYWR5IGEgc3BpbmxvY2sgaW4gdGhlIG1xLWRlYWRsaW5lIHNjaGVkdWxlciB0aGF0
IHNlcmlhbGl6ZXMNCm1vc3Qgb3BlcmF0aW9ucywgZG8gd2UgcmVhbGx5IG5lZWQgYSBuZXcgc3Bp
bmxvY2s/DQoNCj4gIC8qDQo+ICAgKiBXcml0ZSB1bmxvY2sgdGhlIHRhcmdldCB6b25lIG9mIGEg
d3JpdGUgcmVxdWVzdC4NCj4gKyAqIENsZWFyaW5nIHRoZSB0YXJnZXQgem9uZSB3cml0ZSBsb2Nr
IGJpdCBpcyBkb25lIHdpdGggdGhlIHNjaGVkdWxlciB6b25lX2xvY2sNCj4gKyAqIHNwaW5sb2Nr
IGhlbGQgc28gdGhhdCBkZWFkbGluZV9uZXh0X3JlcXVlc3QoKSBhbmQgZGVhZGxpbmVfZmlmb19y
ZXF1ZXN0KCkNCj4gKyAqIGNhbm5vdCBzZWUgdGhlIGxvY2sgc3RhdGUgb2YgYSB6b25lIGNoYW5n
ZSBkdWUgdG8gYSByZXF1ZXN0IGNvbXBsZXRpb24gZHVyaW5nDQo+ICsgKiB0aGVpciBldmVudHVh
bCBzZWFyY2ggZm9yIGFuIGFwcHJvcHJpYXRlIHdyaXRlIHJlcXVlc3QuIE90aGVyd2lzZSwgZm9y
IGEgem9uZQ0KPiArICogd2l0aCBtdWx0aXBsZSB3cml0ZSByZXF1ZXN0cyBxdWV1ZWQsIGEgbm9u
IHNlcXVlbnRpYWwgd3JpdGUgcmVxdWVzdA0KPiArICogY2FuIGJlIGNob3Nlbi4NCj4gICAqLw0K
PiAgc3RhdGljIHZvaWQgZGVhZGxpbmVfd3VubG9ja196b25lKHN0cnVjdCBkZWFkbGluZV9kYXRh
ICpkZCwNCj4gIAkJCQkgIHN0cnVjdCByZXF1ZXN0ICpycSkNCj4gIHsNCj4gKwl1bnNpZ25lZCBs
b25nIGZsYWdzOw0KPiArDQo+ICsJc3Bpbl9sb2NrX2lycXNhdmUoJmRkLT56b25lX2xvY2ssIGZs
YWdzKTsNCj4gKw0KPiAgCVdBUk5fT05fT05DRSghdGVzdF9hbmRfY2xlYXJfYml0KGJsa19ycV96
b25lX25vKHJxKSwgZGQtPnpvbmVzX3dsb2NrKSk7DQo+ICAJZGVhZGxpbmVfY2xlYXJfcmVxdWVz
dF96b25lX3dsb2NrKHJxKTsNCj4gKw0KPiArCXNwaW5fdW5sb2NrX2lycXJlc3RvcmUoJmRkLT56
b25lX2xvY2ssIGZsYWdzKTsNCj4gIH0NCg0KSXMgYSByZXF1ZXN0IHJlbW92ZWQgZnJvbSB0aGUg
c29ydCBhbmQgZmlmbyBsaXN0cyBiZWZvcmUgYmVpbmcgZGlzcGF0Y2hlZD8gSWYgc28sDQpkb2Vz
IHRoYXQgbWVhbiB0aGF0IG9idGFpbmluZyB6b25lX2xvY2sgaW4gdGhlIGFib3ZlIGZ1bmN0aW9u
IGlzIG5vdCBuZWNlc3Nhcnk/DQoNCj4gIHN0YXRpYyBzdHJ1Y3QgcmVxdWVzdCAqDQo+ICBkZWFk
bGluZV9maWZvX3JlcXVlc3Qoc3RydWN0IGRlYWRsaW5lX2RhdGEgKmRkLCBpbnQgZGF0YV9kaXIp
DQo+ICB7DQo+ICsJc3RydWN0IHJlcXVlc3QgKnJxOw0KPiArCXVuc2lnbmVkIGxvbmcgZmxhZ3M7
DQo+ICsNCj4gIAlpZiAoV0FSTl9PTl9PTkNFKGRhdGFfZGlyICE9IFJFQUQgJiYgZGF0YV9kaXIg
IT0gV1JJVEUpKQ0KPiAgCQlyZXR1cm4gTlVMTDsNCj4gIA0KPiAgCWlmIChsaXN0X2VtcHR5KCZk
ZC0+Zmlmb19saXN0W2RhdGFfZGlyXSkpDQo+ICAJCXJldHVybiBOVUxMOw0KPiAgDQo+IC0JcmV0
dXJuIHJxX2VudHJ5X2ZpZm8oZGQtPmZpZm9fbGlzdFtkYXRhX2Rpcl0ubmV4dCk7DQo+ICsJaWYg
KCFkZC0+em9uZXNfd2xvY2sgfHwgZGF0YV9kaXIgPT0gUkVBRCkNCj4gKwkJcmV0dXJuIHJxX2Vu
dHJ5X2ZpZm8oZGQtPmZpZm9fbGlzdFtkYXRhX2Rpcl0ubmV4dCk7DQo+ICsNCj4gKwlzcGluX2xv
Y2tfaXJxc2F2ZSgmZGQtPnpvbmVfbG9jaywgZmxhZ3MpOw0KPiArDQo+ICsJbGlzdF9mb3JfZWFj
aF9lbnRyeShycSwgJmRkLT5maWZvX2xpc3RbV1JJVEVdLCBxdWV1ZWxpc3QpIHsNCj4gKwkJaWYg
KGRlYWRsaW5lX2Nhbl9kaXNwYXRjaF9yZXF1ZXN0KGRkLCBycSkpDQo+ICsJCQlnb3RvIG91dDsN
Cj4gKwl9DQo+ICsJcnEgPSBOVUxMOw0KPiArDQo+ICtvdXQ6DQo+ICsJc3Bpbl91bmxvY2tfaXJx
cmVzdG9yZSgmZGQtPnpvbmVfbG9jaywgZmxhZ3MpOw0KPiArDQo+ICsJcmV0dXJuIHJxOw0KPiAg
fQ0KDQpTYW1lIHF1ZXN0aW9uIGhlcmU6IGlzIGl0IHJlYWxseSBuZWNlc3NhcnkgdG8gb2J0YWlu
IHpvbmVfbG9jaz8NCg0KVGhhbmtzLA0KDQpCYXJ0Lg==
WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
Damien Le Moal <Damien.LeMoal@wdc.com>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
"axboe@kernel.dk" <axboe@kernel.dk>
Cc: "hch@lst.de" <hch@lst.de>
Subject: Re: [PATCH V6 13/14] block: mq-deadline: Limit write request dispatch for zoned block devices
Date: Mon, 2 Oct 2017 23:44:07 +0000 [thread overview]
Message-ID: <1506987846.3368.69.camel@wdc.com> (raw)
In-Reply-To: <20171002071535.8007-14-damien.lemoal@wdc.com>
On Mon, 2017-10-02 at 16:15 +0900, Damien Le Moal wrote:
> When dispatching write requests to a zoned block device, only allow
> requests targeting an unlocked zone. Requests targeting a locked zone
> are left in the scheduler queue to preserve the initial write order.
> If no write request can be dispatched, allow reads to be dispatched
> even if the write batch is not done.
>
> To ensure that the search for an appropriate write request is atomic
> in deadline_fifo_request() and deadline_next_request() with reagrd to
^^^^^^
regard?
> write requests zone lock state, introduce the spinlock zone_lock.
> Holding this lock while doing the search in these functions as well as
> when unlocking the target zone of a completed write request in
> dd_completed_request() ensure that the search does not pickup a write
> request in the middle of a zone queued write sequence.
Since there is already a spinlock in the mq-deadline scheduler that serializes
most operations, do we really need a new spinlock?
> /*
> * Write unlock the target zone of a write request.
> + * Clearing the target zone write lock bit is done with the scheduler zone_lock
> + * spinlock held so that deadline_next_request() and deadline_fifo_request()
> + * cannot see the lock state of a zone change due to a request completion during
> + * their eventual search for an appropriate write request. Otherwise, for a zone
> + * with multiple write requests queued, a non sequential write request
> + * can be chosen.
> */
> static void deadline_wunlock_zone(struct deadline_data *dd,
> struct request *rq)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dd->zone_lock, flags);
> +
> WARN_ON_ONCE(!test_and_clear_bit(blk_rq_zone_no(rq), dd->zones_wlock));
> deadline_clear_request_zone_wlock(rq);
> +
> + spin_unlock_irqrestore(&dd->zone_lock, flags);
> }
Is a request removed from the sort and fifo lists before being dispatched? If so,
does that mean that obtaining zone_lock in the above function is not necessary?
> static struct request *
> deadline_fifo_request(struct deadline_data *dd, int data_dir)
> {
> + struct request *rq;
> + unsigned long flags;
> +
> if (WARN_ON_ONCE(data_dir != READ && data_dir != WRITE))
> return NULL;
>
> if (list_empty(&dd->fifo_list[data_dir]))
> return NULL;
>
> - return rq_entry_fifo(dd->fifo_list[data_dir].next);
> + if (!dd->zones_wlock || data_dir == READ)
> + return rq_entry_fifo(dd->fifo_list[data_dir].next);
> +
> + spin_lock_irqsave(&dd->zone_lock, flags);
> +
> + list_for_each_entry(rq, &dd->fifo_list[WRITE], queuelist) {
> + if (deadline_can_dispatch_request(dd, rq))
> + goto out;
> + }
> + rq = NULL;
> +
> +out:
> + spin_unlock_irqrestore(&dd->zone_lock, flags);
> +
> + return rq;
> }
Same question here: is it really necessary to obtain zone_lock?
Thanks,
Bart.
next prev parent reply other threads:[~2017-10-02 23:44 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-02 7:15 [PATCH V6 00/14] scsi-mq support for ZBC disks Damien Le Moal
2017-10-02 7:15 ` [PATCH V6 01/14] scsi: sd_zbc: Move ZBC declarations to scsi_proto.h Damien Le Moal
2017-10-02 7:15 ` [PATCH V6 02/14] scsi: sd_zbc: Fix comments and indentation Damien Le Moal
2017-10-02 16:50 ` Bart Van Assche
2017-10-02 16:50 ` Bart Van Assche
2017-10-02 7:15 ` [PATCH V6 03/14] scsi: sd_zbc: Rearrange code Damien Le Moal
2017-10-02 7:15 ` [PATCH V6 04/14] scsi: sd_zbc: Use well defined macros Damien Le Moal
2017-10-02 7:15 ` [PATCH V6 05/14] scsi: sd_zbc: Fix sd_zbc_read_zoned_characteristics() Damien Le Moal
2017-10-02 7:15 ` [PATCH V6 06/14] block: Add zoned block device information to request queue Damien Le Moal
2017-10-02 7:15 ` [PATCH V6 07/14] scsi: sd_zbc: Initialize device request queue zoned data Damien Le Moal
2017-10-02 16:57 ` Bart Van Assche
2017-10-02 16:57 ` Bart Van Assche
2017-10-02 7:15 ` [PATCH V6 08/14] scsi: sd_zbc: Limit zone write locking to sequential zones Damien Le Moal
2017-10-02 17:00 ` Bart Van Assche
2017-10-02 17:00 ` Bart Van Assche
2017-10-02 7:15 ` [PATCH V6 09/14] scsi: sd_zbc: Disable zone write locking with scsi-mq Damien Le Moal
2017-10-02 7:15 ` [PATCH V6 10/14] block: mq-deadline: Add zoned block device data Damien Le Moal
2017-10-02 23:06 ` Bart Van Assche
2017-10-02 23:06 ` Bart Van Assche
2017-10-02 7:15 ` [PATCH V6 11/14] blokc: mq-deadline: Introduce dispatch helpers Damien Le Moal
2017-10-02 7:15 ` [PATCH V6 12/14] block: mq-deadline: Introduce zone locking support Damien Le Moal
2017-10-02 23:12 ` Bart Van Assche
2017-10-02 23:12 ` Bart Van Assche
2017-10-02 7:15 ` [PATCH V6 13/14] block: mq-deadline: Limit write request dispatch for zoned block devices Damien Le Moal
2017-10-02 23:44 ` Bart Van Assche [this message]
2017-10-02 23:44 ` Bart Van Assche
2017-10-03 0:19 ` Damien Le Moal
2017-10-03 20:56 ` Bart Van Assche
2017-10-03 20:56 ` Bart Van Assche
2017-10-03 23:03 ` Damien Le Moal
2017-10-02 7:15 ` [PATCH V6 14/14] block: do not set mq default scheduler Damien Le Moal
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=1506987846.3368.69.camel@wdc.com \
--to=bart.vanassche@wdc.com \
--cc=Damien.LeMoal@wdc.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=linux-block@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
/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.