diff for duplicates of <1507064168.2567.33.camel@wdc.com> diff --git a/a/1.txt b/N1/1.txt index 180fa46..6094160 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,43 +1,52 @@ -T24gVHVlLCAyMDE3LTEwLTAzIGF0IDA5OjE5ICswOTAwLCBEYW1pZW4gTGUgTW9hbCB3cm90ZToN -Cj4gT24gMTAvMy8xNyAwODo0NCwgQmFydCBWYW4gQXNzY2hlIHdyb3RlOg0KPiA+IE9uIE1vbiwg -MjAxNy0xMC0wMiBhdCAxNjoxNSArMDkwMCwgRGFtaWVuIExlIE1vYWwgd3JvdGU6DQo+ID4gPiAg -c3RhdGljIHZvaWQgZGVhZGxpbmVfd3VubG9ja196b25lKHN0cnVjdCBkZWFkbGluZV9kYXRhICpk -ZCwNCj4gPiA+ICAJCQkJICBzdHJ1Y3QgcmVxdWVzdCAqcnEpDQo+ID4gPiAgew0KPiA+ID4gKwl1 -bnNpZ25lZCBsb25nIGZsYWdzOw0KPiA+ID4gKw0KPiA+ID4gKwlzcGluX2xvY2tfaXJxc2F2ZSgm -ZGQtPnpvbmVfbG9jaywgZmxhZ3MpOw0KPiA+ID4gKw0KPiA+ID4gIAlXQVJOX09OX09OQ0UoIXRl -c3RfYW5kX2NsZWFyX2JpdChibGtfcnFfem9uZV9ubyhycSksIGRkLT56b25lc193bG9jaykpOw0K -PiA+ID4gIAlkZWFkbGluZV9jbGVhcl9yZXF1ZXN0X3pvbmVfd2xvY2socnEpOw0KPiA+ID4gKw0K -PiA+ID4gKwlzcGluX3VubG9ja19pcnFyZXN0b3JlKCZkZC0+em9uZV9sb2NrLCBmbGFncyk7DQo+ -ID4gPiAgfQ0KPiA+IA0KPiA+IElzIGEgcmVxdWVzdCByZW1vdmVkIGZyb20gdGhlIHNvcnQgYW5k -IGZpZm8gbGlzdHMgYmVmb3JlIGJlaW5nIGRpc3BhdGNoZWQ/IElmIHNvLA0KPiA+IGRvZXMgdGhh -dCBtZWFuIHRoYXQgb2J0YWluaW5nIHpvbmVfbG9jayBpbiB0aGUgYWJvdmUgZnVuY3Rpb24gaXMg -bm90IG5lY2Vzc2FyeT8NCj4gDQo+IFllcywgYSByZXF1ZXN0IGlzIHJlbW92ZWQgZnJvbSB0aGUg -c29ydCB0cmVlIGFuZCBmaWZvIGxpc3QgYmVmb3JlDQo+IGRpc3BhdGNoaW5nLiBCdXQgdGhlIGRk -LT56b25lX2xvY2sgc3BpbmxvY2sgaXMgbm90IHRoZXJlIHRvIHByb3RlY3QNCj4gdGhhdCwgZGQt -PmxvY2sgcHJvdGVjdHMgdGhlIHNvcnQgdHJlZSBhbmQgZmlmbyBsaXN0LiBkZC0+em9uZV9sb2Nr -IHdhcw0KPiBhZGRlZCB0byBwcmV2ZW50IHRoZSBjb21wbGV0ZWRfcmVxdWVzdCgpIG1ldGhvZCBm -cm9tIGNoYW5naW5nIGEgem9uZQ0KPiBsb2NrIHN0YXRlIHdoaWxlIGRlYWRsaW5lX2ZpZm9fcmVx -dWVzdHMoKSBvciBkZWFkbGluZV9uZXh0X3JlcXVlc3QoKSBhcmUNCj4gcnVubmluZy4gRXg6DQo+ -IA0KPiBJbWFnaW5lIHRoaXMgY2FzZTogd3JpdGUgcmVxdWVzdCBBIGZvciBhIHpvbmUgWiBpcyBi -ZWluZyBleGVjdXRlZCAoaXQNCj4gd2FzIGRpc3BhdGNoZWQpIHNvIFogaXMgbG9ja2VkLiBEaXNw -YXRjaCBydW5zIGFuZCBpbnNwZWN0cyB0aGUgbmV4dA0KPiByZXF1ZXN0cyBpbiBzb3J0IG9yZGVy -LiBMZXQgc2F5IHdlIGhhdmUgdGhlIHNlcXVlbnRpYWwgd3JpdGVzIEIsIEMsIEQsIEUNCj4gcXVl -dWVkIGZvciB0aGUgc2FtZSB6b25lIFouIEZpcnN0IEIgaXMgaW5zcGVjdGVkIGFuZCBjYW5ub3Qg -YmUNCj4gZGlzcGF0Y2hlZCAoem9uZSBsb2NrZWQpLiBJbnNwZWN0aW9uIG1vdmUgb24gdG8gQywg -YnV0IGJlZm9yZSB0aGUgdGhhdCwNCj4gQSBjb21wbGV0ZXMgYW5kIFogaXMgdW5sb2NrZWQuIFRo -ZW4gQyB3aWxsIGJlIE9LIHRvIGdvIGFzIHRoZSB6b25lIGlzDQo+IG5vdyB1bmxvY2tlZC4gQnV0 -IGl0IGlzIHRoZSB3cm9uZyBjaG9pY2UgYXMgaXQgd2lsbCByZXN1bHQgaW4gb3V0IG9mDQo+IG9y -ZGVyIHdyaXRlLiBCIG11c3QgYmUgdGhlIG5leHQgcmVxdWVzdCBkaXNwYXRjaGVkIGFmdGVyIEEu -DQo+IA0KPiBkZC0+em9uZV9sb2NrIHByZXZlbnRzIHRoaXMgZnJvbSBoYXBwZW5pbmcuIFdpdGhv -dXQgdGhpcyBzcGlubG9jaywgdGhlDQo+IGJhZCBleGFtcGxlIGNhc2UgYWJvdmUgaGFwcGVucyB2 -ZXJ5IGVhc2lseS4NCg0KSGVsbG8gRGFtaWVuLA0KDQpUaGFua3MgZm9yIHRoZSBkZXRhaWxlZCBh -bmQgcmVhbGx5IGNsZWFyIHJlcGx5LiBJIGhvcGUgeW91IGRvIG5vdCBtaW5kIHRoYXQgSQ0KaGF2 -ZSBhIGZldyBmdXJ0aGVyIHF1ZXN0aW9ucyBhYm91dCB0aGlzIHBhdGNoPw0KLSBEb2VzIHRoZSB6 -b25lX2xvY2sgc3BpbmxvY2sgaGF2ZSB0byBiZSBhY3F1aXJlZCBieSBib3RoIGRlYWRsaW5lX3d1 -bmxvY2tfem9uZSgpDQogIGNhbGxlcnMgb3Igb25seSBieSB0aGUgY2FsbCBmcm9tIHRoZSByZXF1 -ZXN0IGNvbXBsZXRpb24gcGF0aD8NCi0gV2h5IGRvIGJvdGggdGhlIG1xLWRlYWRsaW5lIGFuZCB0 -aGUgc2QgZHJpdmVyIGVhY2ggaGF2ZSB0aGVpciBvd24gaW5zdGFuY2Ugb2YNCiAgdGhlIHpvbmVz -X3dsb2NrIGJpdG1hcD8gSGFzIGl0IGJlZW4gY29uc2lkZXJlZCB0byBjb252ZXJ0IGJvdGggYml0 -bWFwcyBpbnRvIGENCiAgc2luZ2xlIGJpdG1hcCB0aGF0IGlzIHNoYXJlZCBieSBib3RoIGtlcm5l -bCBjb21wb25lbnRzIGFuZCB0aGF0IGV4aXN0cyBlLmcuIGF0DQogIHRoZSByZXF1ZXN0IHF1ZXVl -IGxldmVsPw0KDQpUaGFua3MsDQoNCkJhcnQu +On Tue, 2017-10-03 at 09:19 +0900, Damien Le Moal wrote: +> On 10/3/17 08:44, Bart Van Assche wrote: +> > On Mon, 2017-10-02 at 16:15 +0900, Damien Le Moal wrote: +> > > 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? +> +> Yes, a request is removed from the sort tree and fifo list before +> dispatching. But the dd->zone_lock spinlock is not there to protect +> that, dd->lock protects the sort tree and fifo list. dd->zone_lock was +> added to prevent the completed_request() method from changing a zone +> lock state while deadline_fifo_requests() or deadline_next_request() are +> running. Ex: +> +> Imagine this case: write request A for a zone Z is being executed (it +> was dispatched) so Z is locked. Dispatch runs and inspects the next +> requests in sort order. Let say we have the sequential writes B, C, D, E +> queued for the same zone Z. First B is inspected and cannot be +> dispatched (zone locked). Inspection move on to C, but before the that, +> A completes and Z is unlocked. Then C will be OK to go as the zone is +> now unlocked. But it is the wrong choice as it will result in out of +> order write. B must be the next request dispatched after A. +> +> dd->zone_lock prevents this from happening. Without this spinlock, the +> bad example case above happens very easily. + +Hello Damien, + +Thanks for the detailed and really clear reply. I hope you do not mind that I +have a few further questions about this patch? +- Does the zone_lock spinlock have to be acquired by both deadline_wunlock_zone() + callers or only by the call from the request completion path? +- Why do both the mq-deadline and the sd driver each have their own instance of + the zones_wlock bitmap? Has it been considered to convert both bitmaps into a + single bitmap that is shared by both kernel components and that exists e.g. at + the request queue level? + +Thanks, + +Bart. diff --git a/a/content_digest b/N1/content_digest index a9908d9..a9bba7e 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -13,48 +13,57 @@ "Cc\0hch@lst.de <hch@lst.de>\0" "\00:1\0" "b\0" - "T24gVHVlLCAyMDE3LTEwLTAzIGF0IDA5OjE5ICswOTAwLCBEYW1pZW4gTGUgTW9hbCB3cm90ZToN\n" - "Cj4gT24gMTAvMy8xNyAwODo0NCwgQmFydCBWYW4gQXNzY2hlIHdyb3RlOg0KPiA+IE9uIE1vbiwg\n" - "MjAxNy0xMC0wMiBhdCAxNjoxNSArMDkwMCwgRGFtaWVuIExlIE1vYWwgd3JvdGU6DQo+ID4gPiAg\n" - "c3RhdGljIHZvaWQgZGVhZGxpbmVfd3VubG9ja196b25lKHN0cnVjdCBkZWFkbGluZV9kYXRhICpk\n" - "ZCwNCj4gPiA+ICAJCQkJICBzdHJ1Y3QgcmVxdWVzdCAqcnEpDQo+ID4gPiAgew0KPiA+ID4gKwl1\n" - "bnNpZ25lZCBsb25nIGZsYWdzOw0KPiA+ID4gKw0KPiA+ID4gKwlzcGluX2xvY2tfaXJxc2F2ZSgm\n" - "ZGQtPnpvbmVfbG9jaywgZmxhZ3MpOw0KPiA+ID4gKw0KPiA+ID4gIAlXQVJOX09OX09OQ0UoIXRl\n" - "c3RfYW5kX2NsZWFyX2JpdChibGtfcnFfem9uZV9ubyhycSksIGRkLT56b25lc193bG9jaykpOw0K\n" - "PiA+ID4gIAlkZWFkbGluZV9jbGVhcl9yZXF1ZXN0X3pvbmVfd2xvY2socnEpOw0KPiA+ID4gKw0K\n" - "PiA+ID4gKwlzcGluX3VubG9ja19pcnFyZXN0b3JlKCZkZC0+em9uZV9sb2NrLCBmbGFncyk7DQo+\n" - "ID4gPiAgfQ0KPiA+IA0KPiA+IElzIGEgcmVxdWVzdCByZW1vdmVkIGZyb20gdGhlIHNvcnQgYW5k\n" - "IGZpZm8gbGlzdHMgYmVmb3JlIGJlaW5nIGRpc3BhdGNoZWQ/IElmIHNvLA0KPiA+IGRvZXMgdGhh\n" - "dCBtZWFuIHRoYXQgb2J0YWluaW5nIHpvbmVfbG9jayBpbiB0aGUgYWJvdmUgZnVuY3Rpb24gaXMg\n" - "bm90IG5lY2Vzc2FyeT8NCj4gDQo+IFllcywgYSByZXF1ZXN0IGlzIHJlbW92ZWQgZnJvbSB0aGUg\n" - "c29ydCB0cmVlIGFuZCBmaWZvIGxpc3QgYmVmb3JlDQo+IGRpc3BhdGNoaW5nLiBCdXQgdGhlIGRk\n" - "LT56b25lX2xvY2sgc3BpbmxvY2sgaXMgbm90IHRoZXJlIHRvIHByb3RlY3QNCj4gdGhhdCwgZGQt\n" - "PmxvY2sgcHJvdGVjdHMgdGhlIHNvcnQgdHJlZSBhbmQgZmlmbyBsaXN0LiBkZC0+em9uZV9sb2Nr\n" - "IHdhcw0KPiBhZGRlZCB0byBwcmV2ZW50IHRoZSBjb21wbGV0ZWRfcmVxdWVzdCgpIG1ldGhvZCBm\n" - "cm9tIGNoYW5naW5nIGEgem9uZQ0KPiBsb2NrIHN0YXRlIHdoaWxlIGRlYWRsaW5lX2ZpZm9fcmVx\n" - "dWVzdHMoKSBvciBkZWFkbGluZV9uZXh0X3JlcXVlc3QoKSBhcmUNCj4gcnVubmluZy4gRXg6DQo+\n" - "IA0KPiBJbWFnaW5lIHRoaXMgY2FzZTogd3JpdGUgcmVxdWVzdCBBIGZvciBhIHpvbmUgWiBpcyBi\n" - "ZWluZyBleGVjdXRlZCAoaXQNCj4gd2FzIGRpc3BhdGNoZWQpIHNvIFogaXMgbG9ja2VkLiBEaXNw\n" - "YXRjaCBydW5zIGFuZCBpbnNwZWN0cyB0aGUgbmV4dA0KPiByZXF1ZXN0cyBpbiBzb3J0IG9yZGVy\n" - "LiBMZXQgc2F5IHdlIGhhdmUgdGhlIHNlcXVlbnRpYWwgd3JpdGVzIEIsIEMsIEQsIEUNCj4gcXVl\n" - "dWVkIGZvciB0aGUgc2FtZSB6b25lIFouIEZpcnN0IEIgaXMgaW5zcGVjdGVkIGFuZCBjYW5ub3Qg\n" - "YmUNCj4gZGlzcGF0Y2hlZCAoem9uZSBsb2NrZWQpLiBJbnNwZWN0aW9uIG1vdmUgb24gdG8gQywg\n" - "YnV0IGJlZm9yZSB0aGUgdGhhdCwNCj4gQSBjb21wbGV0ZXMgYW5kIFogaXMgdW5sb2NrZWQuIFRo\n" - "ZW4gQyB3aWxsIGJlIE9LIHRvIGdvIGFzIHRoZSB6b25lIGlzDQo+IG5vdyB1bmxvY2tlZC4gQnV0\n" - "IGl0IGlzIHRoZSB3cm9uZyBjaG9pY2UgYXMgaXQgd2lsbCByZXN1bHQgaW4gb3V0IG9mDQo+IG9y\n" - "ZGVyIHdyaXRlLiBCIG11c3QgYmUgdGhlIG5leHQgcmVxdWVzdCBkaXNwYXRjaGVkIGFmdGVyIEEu\n" - "DQo+IA0KPiBkZC0+em9uZV9sb2NrIHByZXZlbnRzIHRoaXMgZnJvbSBoYXBwZW5pbmcuIFdpdGhv\n" - "dXQgdGhpcyBzcGlubG9jaywgdGhlDQo+IGJhZCBleGFtcGxlIGNhc2UgYWJvdmUgaGFwcGVucyB2\n" - "ZXJ5IGVhc2lseS4NCg0KSGVsbG8gRGFtaWVuLA0KDQpUaGFua3MgZm9yIHRoZSBkZXRhaWxlZCBh\n" - "bmQgcmVhbGx5IGNsZWFyIHJlcGx5LiBJIGhvcGUgeW91IGRvIG5vdCBtaW5kIHRoYXQgSQ0KaGF2\n" - "ZSBhIGZldyBmdXJ0aGVyIHF1ZXN0aW9ucyBhYm91dCB0aGlzIHBhdGNoPw0KLSBEb2VzIHRoZSB6\n" - "b25lX2xvY2sgc3BpbmxvY2sgaGF2ZSB0byBiZSBhY3F1aXJlZCBieSBib3RoIGRlYWRsaW5lX3d1\n" - "bmxvY2tfem9uZSgpDQogIGNhbGxlcnMgb3Igb25seSBieSB0aGUgY2FsbCBmcm9tIHRoZSByZXF1\n" - "ZXN0IGNvbXBsZXRpb24gcGF0aD8NCi0gV2h5IGRvIGJvdGggdGhlIG1xLWRlYWRsaW5lIGFuZCB0\n" - "aGUgc2QgZHJpdmVyIGVhY2ggaGF2ZSB0aGVpciBvd24gaW5zdGFuY2Ugb2YNCiAgdGhlIHpvbmVz\n" - "X3dsb2NrIGJpdG1hcD8gSGFzIGl0IGJlZW4gY29uc2lkZXJlZCB0byBjb252ZXJ0IGJvdGggYml0\n" - "bWFwcyBpbnRvIGENCiAgc2luZ2xlIGJpdG1hcCB0aGF0IGlzIHNoYXJlZCBieSBib3RoIGtlcm5l\n" - "bCBjb21wb25lbnRzIGFuZCB0aGF0IGV4aXN0cyBlLmcuIGF0DQogIHRoZSByZXF1ZXN0IHF1ZXVl\n" - IGxldmVsPw0KDQpUaGFua3MsDQoNCkJhcnQu + "On Tue, 2017-10-03 at 09:19 +0900, Damien Le Moal wrote:\n" + "> On 10/3/17 08:44, Bart Van Assche wrote:\n" + "> > On Mon, 2017-10-02 at 16:15 +0900, Damien Le Moal wrote:\n" + "> > > static void deadline_wunlock_zone(struct deadline_data *dd,\n" + "> > > \t\t\t\t struct request *rq)\n" + "> > > {\n" + "> > > +\tunsigned long flags;\n" + "> > > +\n" + "> > > +\tspin_lock_irqsave(&dd->zone_lock, flags);\n" + "> > > +\n" + "> > > \tWARN_ON_ONCE(!test_and_clear_bit(blk_rq_zone_no(rq), dd->zones_wlock));\n" + "> > > \tdeadline_clear_request_zone_wlock(rq);\n" + "> > > +\n" + "> > > +\tspin_unlock_irqrestore(&dd->zone_lock, flags);\n" + "> > > }\n" + "> > \n" + "> > Is a request removed from the sort and fifo lists before being dispatched? If so,\n" + "> > does that mean that obtaining zone_lock in the above function is not necessary?\n" + "> \n" + "> Yes, a request is removed from the sort tree and fifo list before\n" + "> dispatching. But the dd->zone_lock spinlock is not there to protect\n" + "> that, dd->lock protects the sort tree and fifo list. dd->zone_lock was\n" + "> added to prevent the completed_request() method from changing a zone\n" + "> lock state while deadline_fifo_requests() or deadline_next_request() are\n" + "> running. Ex:\n" + "> \n" + "> Imagine this case: write request A for a zone Z is being executed (it\n" + "> was dispatched) so Z is locked. Dispatch runs and inspects the next\n" + "> requests in sort order. Let say we have the sequential writes B, C, D, E\n" + "> queued for the same zone Z. First B is inspected and cannot be\n" + "> dispatched (zone locked). Inspection move on to C, but before the that,\n" + "> A completes and Z is unlocked. Then C will be OK to go as the zone is\n" + "> now unlocked. But it is the wrong choice as it will result in out of\n" + "> order write. B must be the next request dispatched after A.\n" + "> \n" + "> dd->zone_lock prevents this from happening. Without this spinlock, the\n" + "> bad example case above happens very easily.\n" + "\n" + "Hello Damien,\n" + "\n" + "Thanks for the detailed and really clear reply. I hope you do not mind that I\n" + "have a few further questions about this patch?\n" + "- Does the zone_lock spinlock have to be acquired by both deadline_wunlock_zone()\n" + " callers or only by the call from the request completion path?\n" + "- Why do both the mq-deadline and the sd driver each have their own instance of\n" + " the zones_wlock bitmap? Has it been considered to convert both bitmaps into a\n" + " single bitmap that is shared by both kernel components and that exists e.g. at\n" + " the request queue level?\n" + "\n" + "Thanks,\n" + "\n" + Bart. -9cebc5e553951d5923be4ea8a9683520c0dbfebd7c88a3d36426a34828952093 +eae89eac298719f95be01138371763bb35828d8db8279e821f05d84b70506817
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.