All of lore.kernel.org
 help / color / mirror / Atom feed
diff for duplicates of <6db16aa3a7c56b6dcca2d10b4e100a780c740081.camel@wdc.com>

diff --git a/a/1.txt b/N1/1.txt
index 7f158f5..f5ee908 100644
--- a/a/1.txt
+++ b/N1/1.txt
@@ -1,41 +1,51 @@
-T24gV2VkLCAyMDE4LTA1LTE2IGF0IDE3OjE2ICswMjAwLCBEbWl0cnkgVnl1a292IHdyb3RlOg0K
-PiBPbiBXZWQsIE1heSAxNiwgMjAxOCBhdCA0OjU2IFBNLCBCYXJ0IFZhbiBBc3NjaGUgPEJhcnQu
-VmFuQXNzY2hlQHdkYy5jb20+IHdyb3RlOg0KPiA+IE9uIFdlZCwgMjAxOC0wNS0xNiBhdCAyMjow
-NSArMDkwMCwgVGV0c3VvIEhhbmRhIHdyb3RlOg0KPiA+ID4gZGlmZiAtLWdpdCBhL2Jsb2NrL2Js
-ay1jb3JlLmMgYi9ibG9jay9ibGstY29yZS5jDQo+ID4gPiBpbmRleCA4NTkwOWI0Li41OWUyNDk2
-IDEwMDY0NA0KPiA+ID4gLS0tIGEvYmxvY2svYmxrLWNvcmUuYw0KPiA+ID4gKysrIGIvYmxvY2sv
-YmxrLWNvcmUuYw0KPiA+ID4gQEAgLTk1MSwxMCArOTUxLDEwIEBAIGludCBibGtfcXVldWVfZW50
-ZXIoc3RydWN0IHJlcXVlc3RfcXVldWUgKnEsIGJsa19tcV9yZXFfZmxhZ3NfdCBmbGFncykNCj4g
-PiA+ICAgICAgICAgICAgICAgc21wX3JtYigpOw0KPiA+ID4gDQo+ID4gPiAgICAgICAgICAgICAg
-IHdhaXRfZXZlbnQocS0+bXFfZnJlZXplX3dxLA0KPiA+ID4gLSAgICAgICAgICAgICAgICAgICAg
-ICAgIChhdG9taWNfcmVhZCgmcS0+bXFfZnJlZXplX2RlcHRoKSA9PSAwICYmDQo+ID4gPiAtICAg
-ICAgICAgICAgICAgICAgICAgICAgIChwcmVlbXB0IHx8ICFibGtfcXVldWVfcHJlZW1wdF9vbmx5
-KHEpKSkgfHwNCj4gPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICBhdG9taWNfcmVhZCgmcS0+
-bXFfZnJlZXplX2RlcHRoKSB8fA0KPiA+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgIChwcmVl
-bXB0IHx8ICFibGtfcXVldWVfcHJlZW1wdF9vbmx5KHEpKSB8fA0KPiA+ID4gICAgICAgICAgICAg
-ICAgICAgICAgICAgIGJsa19xdWV1ZV9keWluZyhxKSk7DQo+ID4gPiAtICAgICAgICAgICAgIGlm
-IChibGtfcXVldWVfZHlpbmcocSkpDQo+ID4gPiArICAgICAgICAgICAgIGlmIChhdG9taWNfcmVh
-ZCgmcS0+bXFfZnJlZXplX2RlcHRoKSB8fCBibGtfcXVldWVfZHlpbmcocSkpDQo+ID4gPiAgICAg
-ICAgICAgICAgICAgICAgICAgcmV0dXJuIC1FTk9ERVY7DQo+ID4gPiAgICAgICB9DQo+ID4gPiAg
-fQ0KPiA+IA0KPiA+IFRoYXQgY2hhbmdlIGxvb2tzIHdyb25nIHRvIG1lLg0KPiANCj4gSGkgQmFy
-dCwNCj4gDQo+IFdoeSBkb2VzIGl0IGxvb2sgd3JvbmcgdG8geW91Pw0KDQpCZWNhdXNlIHRoYXQg
-Y2hhbmdlIGNvbmZsaWN0cyB3aXRoIHRoZSBwdXJwb3NlIG9mIHF1ZXVlIGZyZWV6aW5nIGFuZCBh
-bHNvIGJlY2F1c2UNCnRoYXQgY2hhbmdlIHdvdWxkIGluamVjdCBJL08gZXJyb3JzIGluIGNvZGUg
-cGF0aHMgdGhhdCBzaG91bGRuJ3QgaW5qZWN0IEkvTyBlcnJvcnMuDQpQbGVhc2UgaGF2ZSBhIGxv
-b2sgYXQgZS5nLiBnZW5lcmljX21ha2VfcmVxdWVzdCgpLiBGcm9tIHRoZSBzdGFydCBvZiB0aGF0
-IGZ1bmN0aW9uOg0KDQoJaWYgKGJsa19xdWV1ZV9lbnRlcihxLCBmbGFncykgPCAwKSB7DQoJCWlm
-ICghYmxrX3F1ZXVlX2R5aW5nKHEpICYmIChiaW8tPmJpX29wZiAmIFJFUV9OT1dBSVQpKQ0KCQkJ
-YmlvX3dvdWxkYmxvY2tfZXJyb3IoYmlvKTsNCgkJZWxzZQ0KCQkJYmlvX2lvX2Vycm9yKGJpbyk7
-DQoJCXJldHVybiByZXQ7DQoJfQ0KDQpUaGUgYWJvdmUgcGF0Y2ggY2hhbmdlcyB0aGUgYmVoYXZp
-b3Igb2YgYmxrX3F1ZXVlX2VudGVyKCkgY29kZSBmcm9tIHdhaXRpbmcgd2hpbGUNCnEtPm1xX2Zy
-ZWV6ZV9kZXB0aCAhPSAwIGludG8gcmV0dXJuaW5nIC1FTk9ERVYgd2hpbGUgdGhlIHJlcXVlc3Qg
-cXVldWUgaXMgZnJvemVuLg0KVGhhdCB3aWxsIGNhdXNlIGdlbmVyaWNfbWFrZV9yZXF1ZXN0KCkg
-dG8gY2FsbCBiaW9faW9fZXJyb3IoYmlvKSB3aGlsZSBhIHJlcXVlc3QNCnF1ZXVlIGlzIGZyb3pl
-biBpZiBSRVFfTk9XQUlUIGhhcyBub3QgYmVlbiBzZXQsIHdoaWNoIGlzIHRoZSBkZWZhdWx0IGJl
-aGF2aW9yLiBTbw0KYW55IG9wZXJhdGlvbiB0aGF0IGZyZWV6ZXMgdGhlIHF1ZXVlIHRlbXBvcmFy
-aWx5LCBlLmcuIGNoYW5naW5nIHRoZSBxdWV1ZSBkZXB0aCwNCmNvbmN1cnJlbnRseSB3aXRoIEkv
-TyBwcm9jZXNzaW5nIGNhbiBjYXVzZSBJL08gdG8gZmFpbCB3aXRoIC1FTk9ERVYuIEFzIHlvdQ0K
-cHJvYmFibHkga25vdyBmYWlsdXJlIG9mIHdyaXRlIHJlcXVlc3RzIGhhcyB2ZXJ5IGFubm95aW5n
-IGNvbnNlcXVlbmNlcy4gSXQgZS5nLg0KY2F1c2VzIGZpbGVzeXN0ZW1zIHRvIGdvIGludG8gcmVh
-ZC1vbmx5IG1vZGUuIFRoYXQncyB3aHkgSSB0aGluayB0aGF0IHRoZSBhYm92ZQ0KY2hhbmdlIGlz
-IGNvbXBsZXRlbHkgd3JvbmcuDQoNCkJhcnQuDQoNCg0K
\ No newline at end of file
+On Wed, 2018-05-16 at 17:16 +0200, Dmitry Vyukov wrote:
+> On Wed, May 16, 2018 at 4:56 PM, Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
+> > On Wed, 2018-05-16 at 22:05 +0900, Tetsuo Handa wrote:
+> > > diff --git a/block/blk-core.c b/block/blk-core.c
+> > > index 85909b4..59e2496 100644
+> > > --- a/block/blk-core.c
+> > > +++ b/block/blk-core.c
+> > > @@ -951,10 +951,10 @@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags)
+> > >               smp_rmb();
+> > > 
+> > >               wait_event(q->mq_freeze_wq,
+> > > -                        (atomic_read(&q->mq_freeze_depth) == 0 &&
+> > > -                         (preempt || !blk_queue_preempt_only(q))) ||
+> > > +                        atomic_read(&q->mq_freeze_depth) ||
+> > > +                        (preempt || !blk_queue_preempt_only(q)) ||
+> > >                          blk_queue_dying(q));
+> > > -             if (blk_queue_dying(q))
+> > > +             if (atomic_read(&q->mq_freeze_depth) || blk_queue_dying(q))
+> > >                       return -ENODEV;
+> > >       }
+> > >  }
+> > 
+> > That change looks wrong to me.
+> 
+> Hi Bart,
+> 
+> Why does it look wrong to you?
+
+Because that change conflicts with the purpose of queue freezing and also because
+that change would inject I/O errors in code paths that shouldn't inject I/O errors.
+Please have a look at e.g. generic_make_request(). From the start of that function:
+
+	if (blk_queue_enter(q, flags) < 0) {
+		if (!blk_queue_dying(q) && (bio->bi_opf & REQ_NOWAIT))
+			bio_wouldblock_error(bio);
+		else
+			bio_io_error(bio);
+		return ret;
+	}
+
+The above patch changes the behavior of blk_queue_enter() code from waiting while
+q->mq_freeze_depth != 0 into returning -ENODEV while the request queue is frozen.
+That will cause generic_make_request() to call bio_io_error(bio) while a request
+queue is frozen if REQ_NOWAIT has not been set, which is the default behavior. So
+any operation that freezes the queue temporarily, e.g. changing the queue depth,
+concurrently with I/O processing can cause I/O to fail with -ENODEV. As you
+probably know failure of write requests has very annoying consequences. It e.g.
+causes filesystems to go into read-only mode. That's why I think that the above
+change is completely wrong.
+
+Bart.
\ No newline at end of file
diff --git a/a/content_digest b/N1/content_digest
index b82c8fc..cec4dbc 100644
--- a/a/content_digest
+++ b/N1/content_digest
@@ -52,47 +52,57 @@
   "b\0"
 ]
 [
-  "T24gV2VkLCAyMDE4LTA1LTE2IGF0IDE3OjE2ICswMjAwLCBEbWl0cnkgVnl1a292IHdyb3RlOg0K\n",
-  "PiBPbiBXZWQsIE1heSAxNiwgMjAxOCBhdCA0OjU2IFBNLCBCYXJ0IFZhbiBBc3NjaGUgPEJhcnQu\n",
-  "VmFuQXNzY2hlQHdkYy5jb20+IHdyb3RlOg0KPiA+IE9uIFdlZCwgMjAxOC0wNS0xNiBhdCAyMjow\n",
-  "NSArMDkwMCwgVGV0c3VvIEhhbmRhIHdyb3RlOg0KPiA+ID4gZGlmZiAtLWdpdCBhL2Jsb2NrL2Js\n",
-  "ay1jb3JlLmMgYi9ibG9jay9ibGstY29yZS5jDQo+ID4gPiBpbmRleCA4NTkwOWI0Li41OWUyNDk2\n",
-  "IDEwMDY0NA0KPiA+ID4gLS0tIGEvYmxvY2svYmxrLWNvcmUuYw0KPiA+ID4gKysrIGIvYmxvY2sv\n",
-  "YmxrLWNvcmUuYw0KPiA+ID4gQEAgLTk1MSwxMCArOTUxLDEwIEBAIGludCBibGtfcXVldWVfZW50\n",
-  "ZXIoc3RydWN0IHJlcXVlc3RfcXVldWUgKnEsIGJsa19tcV9yZXFfZmxhZ3NfdCBmbGFncykNCj4g\n",
-  "PiA+ICAgICAgICAgICAgICAgc21wX3JtYigpOw0KPiA+ID4gDQo+ID4gPiAgICAgICAgICAgICAg\n",
-  "IHdhaXRfZXZlbnQocS0+bXFfZnJlZXplX3dxLA0KPiA+ID4gLSAgICAgICAgICAgICAgICAgICAg\n",
-  "ICAgIChhdG9taWNfcmVhZCgmcS0+bXFfZnJlZXplX2RlcHRoKSA9PSAwICYmDQo+ID4gPiAtICAg\n",
-  "ICAgICAgICAgICAgICAgICAgICAgIChwcmVlbXB0IHx8ICFibGtfcXVldWVfcHJlZW1wdF9vbmx5\n",
-  "KHEpKSkgfHwNCj4gPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICBhdG9taWNfcmVhZCgmcS0+\n",
-  "bXFfZnJlZXplX2RlcHRoKSB8fA0KPiA+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgIChwcmVl\n",
-  "bXB0IHx8ICFibGtfcXVldWVfcHJlZW1wdF9vbmx5KHEpKSB8fA0KPiA+ID4gICAgICAgICAgICAg\n",
-  "ICAgICAgICAgICAgIGJsa19xdWV1ZV9keWluZyhxKSk7DQo+ID4gPiAtICAgICAgICAgICAgIGlm\n",
-  "IChibGtfcXVldWVfZHlpbmcocSkpDQo+ID4gPiArICAgICAgICAgICAgIGlmIChhdG9taWNfcmVh\n",
-  "ZCgmcS0+bXFfZnJlZXplX2RlcHRoKSB8fCBibGtfcXVldWVfZHlpbmcocSkpDQo+ID4gPiAgICAg\n",
-  "ICAgICAgICAgICAgICAgICAgcmV0dXJuIC1FTk9ERVY7DQo+ID4gPiAgICAgICB9DQo+ID4gPiAg\n",
-  "fQ0KPiA+IA0KPiA+IFRoYXQgY2hhbmdlIGxvb2tzIHdyb25nIHRvIG1lLg0KPiANCj4gSGkgQmFy\n",
-  "dCwNCj4gDQo+IFdoeSBkb2VzIGl0IGxvb2sgd3JvbmcgdG8geW91Pw0KDQpCZWNhdXNlIHRoYXQg\n",
-  "Y2hhbmdlIGNvbmZsaWN0cyB3aXRoIHRoZSBwdXJwb3NlIG9mIHF1ZXVlIGZyZWV6aW5nIGFuZCBh\n",
-  "bHNvIGJlY2F1c2UNCnRoYXQgY2hhbmdlIHdvdWxkIGluamVjdCBJL08gZXJyb3JzIGluIGNvZGUg\n",
-  "cGF0aHMgdGhhdCBzaG91bGRuJ3QgaW5qZWN0IEkvTyBlcnJvcnMuDQpQbGVhc2UgaGF2ZSBhIGxv\n",
-  "b2sgYXQgZS5nLiBnZW5lcmljX21ha2VfcmVxdWVzdCgpLiBGcm9tIHRoZSBzdGFydCBvZiB0aGF0\n",
-  "IGZ1bmN0aW9uOg0KDQoJaWYgKGJsa19xdWV1ZV9lbnRlcihxLCBmbGFncykgPCAwKSB7DQoJCWlm\n",
-  "ICghYmxrX3F1ZXVlX2R5aW5nKHEpICYmIChiaW8tPmJpX29wZiAmIFJFUV9OT1dBSVQpKQ0KCQkJ\n",
-  "YmlvX3dvdWxkYmxvY2tfZXJyb3IoYmlvKTsNCgkJZWxzZQ0KCQkJYmlvX2lvX2Vycm9yKGJpbyk7\n",
-  "DQoJCXJldHVybiByZXQ7DQoJfQ0KDQpUaGUgYWJvdmUgcGF0Y2ggY2hhbmdlcyB0aGUgYmVoYXZp\n",
-  "b3Igb2YgYmxrX3F1ZXVlX2VudGVyKCkgY29kZSBmcm9tIHdhaXRpbmcgd2hpbGUNCnEtPm1xX2Zy\n",
-  "ZWV6ZV9kZXB0aCAhPSAwIGludG8gcmV0dXJuaW5nIC1FTk9ERVYgd2hpbGUgdGhlIHJlcXVlc3Qg\n",
-  "cXVldWUgaXMgZnJvemVuLg0KVGhhdCB3aWxsIGNhdXNlIGdlbmVyaWNfbWFrZV9yZXF1ZXN0KCkg\n",
-  "dG8gY2FsbCBiaW9faW9fZXJyb3IoYmlvKSB3aGlsZSBhIHJlcXVlc3QNCnF1ZXVlIGlzIGZyb3pl\n",
-  "biBpZiBSRVFfTk9XQUlUIGhhcyBub3QgYmVlbiBzZXQsIHdoaWNoIGlzIHRoZSBkZWZhdWx0IGJl\n",
-  "aGF2aW9yLiBTbw0KYW55IG9wZXJhdGlvbiB0aGF0IGZyZWV6ZXMgdGhlIHF1ZXVlIHRlbXBvcmFy\n",
-  "aWx5LCBlLmcuIGNoYW5naW5nIHRoZSBxdWV1ZSBkZXB0aCwNCmNvbmN1cnJlbnRseSB3aXRoIEkv\n",
-  "TyBwcm9jZXNzaW5nIGNhbiBjYXVzZSBJL08gdG8gZmFpbCB3aXRoIC1FTk9ERVYuIEFzIHlvdQ0K\n",
-  "cHJvYmFibHkga25vdyBmYWlsdXJlIG9mIHdyaXRlIHJlcXVlc3RzIGhhcyB2ZXJ5IGFubm95aW5n\n",
-  "IGNvbnNlcXVlbmNlcy4gSXQgZS5nLg0KY2F1c2VzIGZpbGVzeXN0ZW1zIHRvIGdvIGludG8gcmVh\n",
-  "ZC1vbmx5IG1vZGUuIFRoYXQncyB3aHkgSSB0aGluayB0aGF0IHRoZSBhYm92ZQ0KY2hhbmdlIGlz\n",
-  "IGNvbXBsZXRlbHkgd3JvbmcuDQoNCkJhcnQuDQoNCg0K"
+  "On Wed, 2018-05-16 at 17:16 +0200, Dmitry Vyukov wrote:\n",
+  "> On Wed, May 16, 2018 at 4:56 PM, Bart Van Assche <Bart.VanAssche\@wdc.com> wrote:\n",
+  "> > On Wed, 2018-05-16 at 22:05 +0900, Tetsuo Handa wrote:\n",
+  "> > > diff --git a/block/blk-core.c b/block/blk-core.c\n",
+  "> > > index 85909b4..59e2496 100644\n",
+  "> > > --- a/block/blk-core.c\n",
+  "> > > +++ b/block/blk-core.c\n",
+  "> > > \@\@ -951,10 +951,10 \@\@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags)\n",
+  "> > >               smp_rmb();\n",
+  "> > > \n",
+  "> > >               wait_event(q->mq_freeze_wq,\n",
+  "> > > -                        (atomic_read(&q->mq_freeze_depth) == 0 &&\n",
+  "> > > -                         (preempt || !blk_queue_preempt_only(q))) ||\n",
+  "> > > +                        atomic_read(&q->mq_freeze_depth) ||\n",
+  "> > > +                        (preempt || !blk_queue_preempt_only(q)) ||\n",
+  "> > >                          blk_queue_dying(q));\n",
+  "> > > -             if (blk_queue_dying(q))\n",
+  "> > > +             if (atomic_read(&q->mq_freeze_depth) || blk_queue_dying(q))\n",
+  "> > >                       return -ENODEV;\n",
+  "> > >       }\n",
+  "> > >  }\n",
+  "> > \n",
+  "> > That change looks wrong to me.\n",
+  "> \n",
+  "> Hi Bart,\n",
+  "> \n",
+  "> Why does it look wrong to you?\n",
+  "\n",
+  "Because that change conflicts with the purpose of queue freezing and also because\n",
+  "that change would inject I/O errors in code paths that shouldn't inject I/O errors.\n",
+  "Please have a look at e.g. generic_make_request(). From the start of that function:\n",
+  "\n",
+  "\tif (blk_queue_enter(q, flags) < 0) {\n",
+  "\t\tif (!blk_queue_dying(q) && (bio->bi_opf & REQ_NOWAIT))\n",
+  "\t\t\tbio_wouldblock_error(bio);\n",
+  "\t\telse\n",
+  "\t\t\tbio_io_error(bio);\n",
+  "\t\treturn ret;\n",
+  "\t}\n",
+  "\n",
+  "The above patch changes the behavior of blk_queue_enter() code from waiting while\n",
+  "q->mq_freeze_depth != 0 into returning -ENODEV while the request queue is frozen.\n",
+  "That will cause generic_make_request() to call bio_io_error(bio) while a request\n",
+  "queue is frozen if REQ_NOWAIT has not been set, which is the default behavior. So\n",
+  "any operation that freezes the queue temporarily, e.g. changing the queue depth,\n",
+  "concurrently with I/O processing can cause I/O to fail with -ENODEV. As you\n",
+  "probably know failure of write requests has very annoying consequences. It e.g.\n",
+  "causes filesystems to go into read-only mode. That's why I think that the above\n",
+  "change is completely wrong.\n",
+  "\n",
+  "Bart."
 ]
 
-1f1c1ba1d933ac21e073b62cd2fc144ebf79cf7184172df00d0c5a1f57b84972
+5bcb2c3d1e7435b03d57dcbabb2ac48f1b35071e40f00d74f70c0334edf6df94

diff --git a/a/1.txt b/N2/1.txt
index 7f158f5..f5ee908 100644
--- a/a/1.txt
+++ b/N2/1.txt
@@ -1,41 +1,51 @@
-T24gV2VkLCAyMDE4LTA1LTE2IGF0IDE3OjE2ICswMjAwLCBEbWl0cnkgVnl1a292IHdyb3RlOg0K
-PiBPbiBXZWQsIE1heSAxNiwgMjAxOCBhdCA0OjU2IFBNLCBCYXJ0IFZhbiBBc3NjaGUgPEJhcnQu
-VmFuQXNzY2hlQHdkYy5jb20+IHdyb3RlOg0KPiA+IE9uIFdlZCwgMjAxOC0wNS0xNiBhdCAyMjow
-NSArMDkwMCwgVGV0c3VvIEhhbmRhIHdyb3RlOg0KPiA+ID4gZGlmZiAtLWdpdCBhL2Jsb2NrL2Js
-ay1jb3JlLmMgYi9ibG9jay9ibGstY29yZS5jDQo+ID4gPiBpbmRleCA4NTkwOWI0Li41OWUyNDk2
-IDEwMDY0NA0KPiA+ID4gLS0tIGEvYmxvY2svYmxrLWNvcmUuYw0KPiA+ID4gKysrIGIvYmxvY2sv
-YmxrLWNvcmUuYw0KPiA+ID4gQEAgLTk1MSwxMCArOTUxLDEwIEBAIGludCBibGtfcXVldWVfZW50
-ZXIoc3RydWN0IHJlcXVlc3RfcXVldWUgKnEsIGJsa19tcV9yZXFfZmxhZ3NfdCBmbGFncykNCj4g
-PiA+ICAgICAgICAgICAgICAgc21wX3JtYigpOw0KPiA+ID4gDQo+ID4gPiAgICAgICAgICAgICAg
-IHdhaXRfZXZlbnQocS0+bXFfZnJlZXplX3dxLA0KPiA+ID4gLSAgICAgICAgICAgICAgICAgICAg
-ICAgIChhdG9taWNfcmVhZCgmcS0+bXFfZnJlZXplX2RlcHRoKSA9PSAwICYmDQo+ID4gPiAtICAg
-ICAgICAgICAgICAgICAgICAgICAgIChwcmVlbXB0IHx8ICFibGtfcXVldWVfcHJlZW1wdF9vbmx5
-KHEpKSkgfHwNCj4gPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICBhdG9taWNfcmVhZCgmcS0+
-bXFfZnJlZXplX2RlcHRoKSB8fA0KPiA+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgIChwcmVl
-bXB0IHx8ICFibGtfcXVldWVfcHJlZW1wdF9vbmx5KHEpKSB8fA0KPiA+ID4gICAgICAgICAgICAg
-ICAgICAgICAgICAgIGJsa19xdWV1ZV9keWluZyhxKSk7DQo+ID4gPiAtICAgICAgICAgICAgIGlm
-IChibGtfcXVldWVfZHlpbmcocSkpDQo+ID4gPiArICAgICAgICAgICAgIGlmIChhdG9taWNfcmVh
-ZCgmcS0+bXFfZnJlZXplX2RlcHRoKSB8fCBibGtfcXVldWVfZHlpbmcocSkpDQo+ID4gPiAgICAg
-ICAgICAgICAgICAgICAgICAgcmV0dXJuIC1FTk9ERVY7DQo+ID4gPiAgICAgICB9DQo+ID4gPiAg
-fQ0KPiA+IA0KPiA+IFRoYXQgY2hhbmdlIGxvb2tzIHdyb25nIHRvIG1lLg0KPiANCj4gSGkgQmFy
-dCwNCj4gDQo+IFdoeSBkb2VzIGl0IGxvb2sgd3JvbmcgdG8geW91Pw0KDQpCZWNhdXNlIHRoYXQg
-Y2hhbmdlIGNvbmZsaWN0cyB3aXRoIHRoZSBwdXJwb3NlIG9mIHF1ZXVlIGZyZWV6aW5nIGFuZCBh
-bHNvIGJlY2F1c2UNCnRoYXQgY2hhbmdlIHdvdWxkIGluamVjdCBJL08gZXJyb3JzIGluIGNvZGUg
-cGF0aHMgdGhhdCBzaG91bGRuJ3QgaW5qZWN0IEkvTyBlcnJvcnMuDQpQbGVhc2UgaGF2ZSBhIGxv
-b2sgYXQgZS5nLiBnZW5lcmljX21ha2VfcmVxdWVzdCgpLiBGcm9tIHRoZSBzdGFydCBvZiB0aGF0
-IGZ1bmN0aW9uOg0KDQoJaWYgKGJsa19xdWV1ZV9lbnRlcihxLCBmbGFncykgPCAwKSB7DQoJCWlm
-ICghYmxrX3F1ZXVlX2R5aW5nKHEpICYmIChiaW8tPmJpX29wZiAmIFJFUV9OT1dBSVQpKQ0KCQkJ
-YmlvX3dvdWxkYmxvY2tfZXJyb3IoYmlvKTsNCgkJZWxzZQ0KCQkJYmlvX2lvX2Vycm9yKGJpbyk7
-DQoJCXJldHVybiByZXQ7DQoJfQ0KDQpUaGUgYWJvdmUgcGF0Y2ggY2hhbmdlcyB0aGUgYmVoYXZp
-b3Igb2YgYmxrX3F1ZXVlX2VudGVyKCkgY29kZSBmcm9tIHdhaXRpbmcgd2hpbGUNCnEtPm1xX2Zy
-ZWV6ZV9kZXB0aCAhPSAwIGludG8gcmV0dXJuaW5nIC1FTk9ERVYgd2hpbGUgdGhlIHJlcXVlc3Qg
-cXVldWUgaXMgZnJvemVuLg0KVGhhdCB3aWxsIGNhdXNlIGdlbmVyaWNfbWFrZV9yZXF1ZXN0KCkg
-dG8gY2FsbCBiaW9faW9fZXJyb3IoYmlvKSB3aGlsZSBhIHJlcXVlc3QNCnF1ZXVlIGlzIGZyb3pl
-biBpZiBSRVFfTk9XQUlUIGhhcyBub3QgYmVlbiBzZXQsIHdoaWNoIGlzIHRoZSBkZWZhdWx0IGJl
-aGF2aW9yLiBTbw0KYW55IG9wZXJhdGlvbiB0aGF0IGZyZWV6ZXMgdGhlIHF1ZXVlIHRlbXBvcmFy
-aWx5LCBlLmcuIGNoYW5naW5nIHRoZSBxdWV1ZSBkZXB0aCwNCmNvbmN1cnJlbnRseSB3aXRoIEkv
-TyBwcm9jZXNzaW5nIGNhbiBjYXVzZSBJL08gdG8gZmFpbCB3aXRoIC1FTk9ERVYuIEFzIHlvdQ0K
-cHJvYmFibHkga25vdyBmYWlsdXJlIG9mIHdyaXRlIHJlcXVlc3RzIGhhcyB2ZXJ5IGFubm95aW5n
-IGNvbnNlcXVlbmNlcy4gSXQgZS5nLg0KY2F1c2VzIGZpbGVzeXN0ZW1zIHRvIGdvIGludG8gcmVh
-ZC1vbmx5IG1vZGUuIFRoYXQncyB3aHkgSSB0aGluayB0aGF0IHRoZSBhYm92ZQ0KY2hhbmdlIGlz
-IGNvbXBsZXRlbHkgd3JvbmcuDQoNCkJhcnQuDQoNCg0K
\ No newline at end of file
+On Wed, 2018-05-16 at 17:16 +0200, Dmitry Vyukov wrote:
+> On Wed, May 16, 2018 at 4:56 PM, Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
+> > On Wed, 2018-05-16 at 22:05 +0900, Tetsuo Handa wrote:
+> > > diff --git a/block/blk-core.c b/block/blk-core.c
+> > > index 85909b4..59e2496 100644
+> > > --- a/block/blk-core.c
+> > > +++ b/block/blk-core.c
+> > > @@ -951,10 +951,10 @@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags)
+> > >               smp_rmb();
+> > > 
+> > >               wait_event(q->mq_freeze_wq,
+> > > -                        (atomic_read(&q->mq_freeze_depth) == 0 &&
+> > > -                         (preempt || !blk_queue_preempt_only(q))) ||
+> > > +                        atomic_read(&q->mq_freeze_depth) ||
+> > > +                        (preempt || !blk_queue_preempt_only(q)) ||
+> > >                          blk_queue_dying(q));
+> > > -             if (blk_queue_dying(q))
+> > > +             if (atomic_read(&q->mq_freeze_depth) || blk_queue_dying(q))
+> > >                       return -ENODEV;
+> > >       }
+> > >  }
+> > 
+> > That change looks wrong to me.
+> 
+> Hi Bart,
+> 
+> Why does it look wrong to you?
+
+Because that change conflicts with the purpose of queue freezing and also because
+that change would inject I/O errors in code paths that shouldn't inject I/O errors.
+Please have a look at e.g. generic_make_request(). From the start of that function:
+
+	if (blk_queue_enter(q, flags) < 0) {
+		if (!blk_queue_dying(q) && (bio->bi_opf & REQ_NOWAIT))
+			bio_wouldblock_error(bio);
+		else
+			bio_io_error(bio);
+		return ret;
+	}
+
+The above patch changes the behavior of blk_queue_enter() code from waiting while
+q->mq_freeze_depth != 0 into returning -ENODEV while the request queue is frozen.
+That will cause generic_make_request() to call bio_io_error(bio) while a request
+queue is frozen if REQ_NOWAIT has not been set, which is the default behavior. So
+any operation that freezes the queue temporarily, e.g. changing the queue depth,
+concurrently with I/O processing can cause I/O to fail with -ENODEV. As you
+probably know failure of write requests has very annoying consequences. It e.g.
+causes filesystems to go into read-only mode. That's why I think that the above
+change is completely wrong.
+
+Bart.
\ No newline at end of file
diff --git a/a/content_digest b/N2/content_digest
index b82c8fc..412c879 100644
--- a/a/content_digest
+++ b/N2/content_digest
@@ -39,11 +39,7 @@
   " oleksandr\@natalenko.name <oleksandr\@natalenko.name>",
   " ming.lei\@redhat.com <ming.lei\@redhat.com>",
   " martin\@lichtvoll.de <martin\@lichtvoll.de>",
-  " hare\@suse.com <hare\@suse.com>",
-  " syzkaller-bugs\@googlegroups.com <syzkaller-bugs\@googlegroups.com>",
-  " ross.zwisler\@linux.intel.com <ross.zwisler\@linux.intel.com>",
-  " keith.busch\@intel.com <keith.busch\@intel.com>",
-  " linux-ext4\@vger.kernel.org <linux-ext4\@vger.kernel.org>\0"
+  " hare\@suse.c\0"
 ]
 [
   "\0000:1\0"
@@ -52,47 +48,57 @@
   "b\0"
 ]
 [
-  "T24gV2VkLCAyMDE4LTA1LTE2IGF0IDE3OjE2ICswMjAwLCBEbWl0cnkgVnl1a292IHdyb3RlOg0K\n",
-  "PiBPbiBXZWQsIE1heSAxNiwgMjAxOCBhdCA0OjU2IFBNLCBCYXJ0IFZhbiBBc3NjaGUgPEJhcnQu\n",
-  "VmFuQXNzY2hlQHdkYy5jb20+IHdyb3RlOg0KPiA+IE9uIFdlZCwgMjAxOC0wNS0xNiBhdCAyMjow\n",
-  "NSArMDkwMCwgVGV0c3VvIEhhbmRhIHdyb3RlOg0KPiA+ID4gZGlmZiAtLWdpdCBhL2Jsb2NrL2Js\n",
-  "ay1jb3JlLmMgYi9ibG9jay9ibGstY29yZS5jDQo+ID4gPiBpbmRleCA4NTkwOWI0Li41OWUyNDk2\n",
-  "IDEwMDY0NA0KPiA+ID4gLS0tIGEvYmxvY2svYmxrLWNvcmUuYw0KPiA+ID4gKysrIGIvYmxvY2sv\n",
-  "YmxrLWNvcmUuYw0KPiA+ID4gQEAgLTk1MSwxMCArOTUxLDEwIEBAIGludCBibGtfcXVldWVfZW50\n",
-  "ZXIoc3RydWN0IHJlcXVlc3RfcXVldWUgKnEsIGJsa19tcV9yZXFfZmxhZ3NfdCBmbGFncykNCj4g\n",
-  "PiA+ICAgICAgICAgICAgICAgc21wX3JtYigpOw0KPiA+ID4gDQo+ID4gPiAgICAgICAgICAgICAg\n",
-  "IHdhaXRfZXZlbnQocS0+bXFfZnJlZXplX3dxLA0KPiA+ID4gLSAgICAgICAgICAgICAgICAgICAg\n",
-  "ICAgIChhdG9taWNfcmVhZCgmcS0+bXFfZnJlZXplX2RlcHRoKSA9PSAwICYmDQo+ID4gPiAtICAg\n",
-  "ICAgICAgICAgICAgICAgICAgICAgIChwcmVlbXB0IHx8ICFibGtfcXVldWVfcHJlZW1wdF9vbmx5\n",
-  "KHEpKSkgfHwNCj4gPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICBhdG9taWNfcmVhZCgmcS0+\n",
-  "bXFfZnJlZXplX2RlcHRoKSB8fA0KPiA+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgIChwcmVl\n",
-  "bXB0IHx8ICFibGtfcXVldWVfcHJlZW1wdF9vbmx5KHEpKSB8fA0KPiA+ID4gICAgICAgICAgICAg\n",
-  "ICAgICAgICAgICAgIGJsa19xdWV1ZV9keWluZyhxKSk7DQo+ID4gPiAtICAgICAgICAgICAgIGlm\n",
-  "IChibGtfcXVldWVfZHlpbmcocSkpDQo+ID4gPiArICAgICAgICAgICAgIGlmIChhdG9taWNfcmVh\n",
-  "ZCgmcS0+bXFfZnJlZXplX2RlcHRoKSB8fCBibGtfcXVldWVfZHlpbmcocSkpDQo+ID4gPiAgICAg\n",
-  "ICAgICAgICAgICAgICAgICAgcmV0dXJuIC1FTk9ERVY7DQo+ID4gPiAgICAgICB9DQo+ID4gPiAg\n",
-  "fQ0KPiA+IA0KPiA+IFRoYXQgY2hhbmdlIGxvb2tzIHdyb25nIHRvIG1lLg0KPiANCj4gSGkgQmFy\n",
-  "dCwNCj4gDQo+IFdoeSBkb2VzIGl0IGxvb2sgd3JvbmcgdG8geW91Pw0KDQpCZWNhdXNlIHRoYXQg\n",
-  "Y2hhbmdlIGNvbmZsaWN0cyB3aXRoIHRoZSBwdXJwb3NlIG9mIHF1ZXVlIGZyZWV6aW5nIGFuZCBh\n",
-  "bHNvIGJlY2F1c2UNCnRoYXQgY2hhbmdlIHdvdWxkIGluamVjdCBJL08gZXJyb3JzIGluIGNvZGUg\n",
-  "cGF0aHMgdGhhdCBzaG91bGRuJ3QgaW5qZWN0IEkvTyBlcnJvcnMuDQpQbGVhc2UgaGF2ZSBhIGxv\n",
-  "b2sgYXQgZS5nLiBnZW5lcmljX21ha2VfcmVxdWVzdCgpLiBGcm9tIHRoZSBzdGFydCBvZiB0aGF0\n",
-  "IGZ1bmN0aW9uOg0KDQoJaWYgKGJsa19xdWV1ZV9lbnRlcihxLCBmbGFncykgPCAwKSB7DQoJCWlm\n",
-  "ICghYmxrX3F1ZXVlX2R5aW5nKHEpICYmIChiaW8tPmJpX29wZiAmIFJFUV9OT1dBSVQpKQ0KCQkJ\n",
-  "YmlvX3dvdWxkYmxvY2tfZXJyb3IoYmlvKTsNCgkJZWxzZQ0KCQkJYmlvX2lvX2Vycm9yKGJpbyk7\n",
-  "DQoJCXJldHVybiByZXQ7DQoJfQ0KDQpUaGUgYWJvdmUgcGF0Y2ggY2hhbmdlcyB0aGUgYmVoYXZp\n",
-  "b3Igb2YgYmxrX3F1ZXVlX2VudGVyKCkgY29kZSBmcm9tIHdhaXRpbmcgd2hpbGUNCnEtPm1xX2Zy\n",
-  "ZWV6ZV9kZXB0aCAhPSAwIGludG8gcmV0dXJuaW5nIC1FTk9ERVYgd2hpbGUgdGhlIHJlcXVlc3Qg\n",
-  "cXVldWUgaXMgZnJvemVuLg0KVGhhdCB3aWxsIGNhdXNlIGdlbmVyaWNfbWFrZV9yZXF1ZXN0KCkg\n",
-  "dG8gY2FsbCBiaW9faW9fZXJyb3IoYmlvKSB3aGlsZSBhIHJlcXVlc3QNCnF1ZXVlIGlzIGZyb3pl\n",
-  "biBpZiBSRVFfTk9XQUlUIGhhcyBub3QgYmVlbiBzZXQsIHdoaWNoIGlzIHRoZSBkZWZhdWx0IGJl\n",
-  "aGF2aW9yLiBTbw0KYW55IG9wZXJhdGlvbiB0aGF0IGZyZWV6ZXMgdGhlIHF1ZXVlIHRlbXBvcmFy\n",
-  "aWx5LCBlLmcuIGNoYW5naW5nIHRoZSBxdWV1ZSBkZXB0aCwNCmNvbmN1cnJlbnRseSB3aXRoIEkv\n",
-  "TyBwcm9jZXNzaW5nIGNhbiBjYXVzZSBJL08gdG8gZmFpbCB3aXRoIC1FTk9ERVYuIEFzIHlvdQ0K\n",
-  "cHJvYmFibHkga25vdyBmYWlsdXJlIG9mIHdyaXRlIHJlcXVlc3RzIGhhcyB2ZXJ5IGFubm95aW5n\n",
-  "IGNvbnNlcXVlbmNlcy4gSXQgZS5nLg0KY2F1c2VzIGZpbGVzeXN0ZW1zIHRvIGdvIGludG8gcmVh\n",
-  "ZC1vbmx5IG1vZGUuIFRoYXQncyB3aHkgSSB0aGluayB0aGF0IHRoZSBhYm92ZQ0KY2hhbmdlIGlz\n",
-  "IGNvbXBsZXRlbHkgd3JvbmcuDQoNCkJhcnQuDQoNCg0K"
+  "On Wed, 2018-05-16 at 17:16 +0200, Dmitry Vyukov wrote:\n",
+  "> On Wed, May 16, 2018 at 4:56 PM, Bart Van Assche <Bart.VanAssche\@wdc.com> wrote:\n",
+  "> > On Wed, 2018-05-16 at 22:05 +0900, Tetsuo Handa wrote:\n",
+  "> > > diff --git a/block/blk-core.c b/block/blk-core.c\n",
+  "> > > index 85909b4..59e2496 100644\n",
+  "> > > --- a/block/blk-core.c\n",
+  "> > > +++ b/block/blk-core.c\n",
+  "> > > \@\@ -951,10 +951,10 \@\@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags)\n",
+  "> > >               smp_rmb();\n",
+  "> > > \n",
+  "> > >               wait_event(q->mq_freeze_wq,\n",
+  "> > > -                        (atomic_read(&q->mq_freeze_depth) == 0 &&\n",
+  "> > > -                         (preempt || !blk_queue_preempt_only(q))) ||\n",
+  "> > > +                        atomic_read(&q->mq_freeze_depth) ||\n",
+  "> > > +                        (preempt || !blk_queue_preempt_only(q)) ||\n",
+  "> > >                          blk_queue_dying(q));\n",
+  "> > > -             if (blk_queue_dying(q))\n",
+  "> > > +             if (atomic_read(&q->mq_freeze_depth) || blk_queue_dying(q))\n",
+  "> > >                       return -ENODEV;\n",
+  "> > >       }\n",
+  "> > >  }\n",
+  "> > \n",
+  "> > That change looks wrong to me.\n",
+  "> \n",
+  "> Hi Bart,\n",
+  "> \n",
+  "> Why does it look wrong to you?\n",
+  "\n",
+  "Because that change conflicts with the purpose of queue freezing and also because\n",
+  "that change would inject I/O errors in code paths that shouldn't inject I/O errors.\n",
+  "Please have a look at e.g. generic_make_request(). From the start of that function:\n",
+  "\n",
+  "\tif (blk_queue_enter(q, flags) < 0) {\n",
+  "\t\tif (!blk_queue_dying(q) && (bio->bi_opf & REQ_NOWAIT))\n",
+  "\t\t\tbio_wouldblock_error(bio);\n",
+  "\t\telse\n",
+  "\t\t\tbio_io_error(bio);\n",
+  "\t\treturn ret;\n",
+  "\t}\n",
+  "\n",
+  "The above patch changes the behavior of blk_queue_enter() code from waiting while\n",
+  "q->mq_freeze_depth != 0 into returning -ENODEV while the request queue is frozen.\n",
+  "That will cause generic_make_request() to call bio_io_error(bio) while a request\n",
+  "queue is frozen if REQ_NOWAIT has not been set, which is the default behavior. So\n",
+  "any operation that freezes the queue temporarily, e.g. changing the queue depth,\n",
+  "concurrently with I/O processing can cause I/O to fail with -ENODEV. As you\n",
+  "probably know failure of write requests has very annoying consequences. It e.g.\n",
+  "causes filesystems to go into read-only mode. That's why I think that the above\n",
+  "change is completely wrong.\n",
+  "\n",
+  "Bart."
 ]
 
-1f1c1ba1d933ac21e073b62cd2fc144ebf79cf7184172df00d0c5a1f57b84972
+2212330979b26905cd74371a6c1dec8984399f7cd6690d1d437310be4adad40a

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.