From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Bart Van Assche To: "hch@lst.de" , "axboe@kernel.dk" CC: "keith.busch@intel.com" , "linux-nvme@lists.infradead.org" , "linux-block@vger.kernel.org" , "sagi@grimberg.me" Subject: Re: [PATCH 10/10] nvme: implement multipath access to nvme subsystems Date: Wed, 23 Aug 2017 18:21:55 +0000 Message-ID: <1503512512.2484.16.camel@wdc.com> References: <20170823175815.3646-1-hch@lst.de> <20170823175815.3646-11-hch@lst.de> In-Reply-To: <20170823175815.3646-11-hch@lst.de> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 List-ID: T24gV2VkLCAyMDE3LTA4LTIzIGF0IDE5OjU4ICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90 ZToNCj4gK3N0YXRpYyBibGtfcWNfdCBudm1lX21ha2VfcmVxdWVzdChzdHJ1Y3QgcmVxdWVzdF9x dWV1ZSAqcSwgc3RydWN0IGJpbyAqYmlvKQ0KPiArew0KPiArCXN0cnVjdCBudm1lX25zX2hlYWQg KmhlYWQgPSBxLT5xdWV1ZWRhdGE7DQo+ICsJc3RydWN0IG52bWVfbnMgKm5zOw0KPiArCWJsa19x Y190IHJldCA9IEJMS19RQ19UX05PTkU7DQo+ICsJaW50IHNyY3VfaWR4Ow0KPiArDQo+ICsJc3Jj dV9pZHggPSBzcmN1X3JlYWRfbG9jaygmaGVhZC0+c3JjdSk7DQo+ICsJbnMgPSBzcmN1X2RlcmVm ZXJlbmNlKGhlYWQtPmN1cnJlbnRfcGF0aCwgJmhlYWQtPnNyY3UpOw0KPiArCWlmICh1bmxpa2Vs eSghbnMgfHwgbnMtPmN0cmwtPnN0YXRlICE9IE5WTUVfQ1RSTF9MSVZFKSkNCj4gKwkJbnMgPSBu dm1lX2ZpbmRfcGF0aChoZWFkKTsNCj4gKwlpZiAobGlrZWx5KG5zKSkgew0KPiArCQliaW8tPmJp X2Rpc2sgPSBucy0+ZGlzazsNCj4gKwkJYmlvLT5iaV9vcGYgfD0gUkVRX0ZBSUxGQVNUX1RSQU5T UE9SVDsNCj4gKwkJcmV0ID0gZ2VuZXJpY19tYWtlX3JlcXVlc3RfZmFzdChiaW8pOw0KPiArCX0g ZWxzZSBpZiAoIWxpc3RfZW1wdHlfY2FyZWZ1bCgmaGVhZC0+bGlzdCkpIHsNCj4gKwkJcHJpbnRr X3JhdGVsaW1pdGVkKCJubyBwYXRoIGF2YWlsYWJsZSAtIHJlcXVlaW5nIEkvT1xuIik7DQo+ICsN Cj4gKwkJc3Bpbl9sb2NrX2lycSgmaGVhZC0+cmVxdWV1ZV9sb2NrKTsNCj4gKwkJYmlvX2xpc3Rf YWRkKCZoZWFkLT5yZXF1ZXVlX2xpc3QsIGJpbyk7DQo+ICsJCXNwaW5fdW5sb2NrX2lycSgmaGVh ZC0+cmVxdWV1ZV9sb2NrKTsNCj4gKwl9IGVsc2Ugew0KPiArCQlwcmludGtfcmF0ZWxpbWl0ZWQo Im5vIHBhdGggLSBmYWlsaW5nIEkvT1xuIik7DQo+ICsNCj4gKwkJYmlvLT5iaV9zdGF0dXMgPSBC TEtfU1RTX0lPRVJSOw0KPiArCQliaW9fZW5kaW8oYmlvKTsNCj4gKwl9DQo+ICsNCj4gKwlzcmN1 X3JlYWRfdW5sb2NrKCZoZWFkLT5zcmN1LCBzcmN1X2lkeCk7DQo+ICsJcmV0dXJuIHJldDsNCj4g K30NCg0KSGVsbG8gQ2hyaXN0b3BoLA0KDQpTaW5jZSBnZW5lcmljX21ha2VfcmVxdWVzdF9mYXN0 KCkgcmV0dXJucyBCTEtfU1RTX0FHQUlOIGZvciBhIGR5aW5nIHBhdGg6DQpjYW4gdGhlIHNhbWUg a2luZCBvZiBzb2Z0IGxvY2t1cHMgb2NjdXIgd2l0aCB0aGUgTlZNZSBtdWx0aXBhdGhpbmcgY29k ZSBhcw0Kd2l0aCB0aGUgY3VycmVudCB1cHN0cmVhbSBkZXZpY2UgbWFwcGVyIG11bHRpcGF0aGlu ZyBjb2RlPyBTZWUgZS5nLg0KIltQQVRDSCAzLzddIGRtLW1wYXRoOiBEbyBub3QgbG9jayB1cCBh IENQVSB3aXRoIHJlcXVldWluZyBhY3Rpdml0eSINCihodHRwczovL3d3dy5yZWRoYXQuY29tL2Fy Y2hpdmVzL2RtLWRldmVsLzIwMTctQXVndXN0L21zZzAwMTI0Lmh0bWwpLg0KDQpBbm90aGVyIHF1 ZXN0aW9uIGFib3V0IHRoaXMgY29kZSBpcyB3aGF0IHdpbGwgaGFwcGVuIGlmDQpnZW5lcmljX21h a2VfcmVxdWVzdF9mYXN0KCkgcmV0dXJucyBCTEtfU1RTX0FHQUlOIGFuZCB0aGUgc3VibWl0X2Jp bygpIG9yDQpnZW5lcmljX21ha2VfcmVxdWVzdCgpIGNhbGxlciBpZ25vcmVzIHRoZSByZXR1cm4g dmFsdWUgb2YgdGhlIGNhbGxlZA0KZnVuY3Rpb24/IEEgcXVpY2sgZ3JlcCByZXZlYWxlZCB0aGF0 IHRoZXJlIGlzIHBsZW50eSBvZiBjb2RlIHRoYXQgaWdub3Jlcw0KdGhlIHJldHVybiB2YWx1ZSBv ZiB0aGVzZSBsYXN0IHR3byBmdW5jdGlvbnMuDQoNClRoYW5rcywNCg0KQmFydC4= From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bart.VanAssche@wdc.com (Bart Van Assche) Date: Wed, 23 Aug 2017 18:21:55 +0000 Subject: [PATCH 10/10] nvme: implement multipath access to nvme subsystems In-Reply-To: <20170823175815.3646-11-hch@lst.de> References: <20170823175815.3646-1-hch@lst.de> <20170823175815.3646-11-hch@lst.de> Message-ID: <1503512512.2484.16.camel@wdc.com> On Wed, 2017-08-23@19:58 +0200, Christoph Hellwig wrote: > +static blk_qc_t nvme_make_request(struct request_queue *q, struct bio *bio) > +{ > + struct nvme_ns_head *head = q->queuedata; > + struct nvme_ns *ns; > + blk_qc_t ret = BLK_QC_T_NONE; > + int srcu_idx; > + > + srcu_idx = srcu_read_lock(&head->srcu); > + ns = srcu_dereference(head->current_path, &head->srcu); > + if (unlikely(!ns || ns->ctrl->state != NVME_CTRL_LIVE)) > + ns = nvme_find_path(head); > + if (likely(ns)) { > + bio->bi_disk = ns->disk; > + bio->bi_opf |= REQ_FAILFAST_TRANSPORT; > + ret = generic_make_request_fast(bio); > + } else if (!list_empty_careful(&head->list)) { > + printk_ratelimited("no path available - requeing I/O\n"); > + > + spin_lock_irq(&head->requeue_lock); > + bio_list_add(&head->requeue_list, bio); > + spin_unlock_irq(&head->requeue_lock); > + } else { > + printk_ratelimited("no path - failing I/O\n"); > + > + bio->bi_status = BLK_STS_IOERR; > + bio_endio(bio); > + } > + > + srcu_read_unlock(&head->srcu, srcu_idx); > + return ret; > +} Hello Christoph, Since generic_make_request_fast() returns BLK_STS_AGAIN for a dying path: can the same kind of soft lockups occur with the NVMe multipathing code as with the current upstream device mapper multipathing code? See e.g. "[PATCH 3/7] dm-mpath: Do not lock up a CPU with requeuing activity" (https://www.redhat.com/archives/dm-devel/2017-August/msg00124.html). Another question about this code is what will happen if generic_make_request_fast() returns BLK_STS_AGAIN and the submit_bio() or generic_make_request() caller ignores the return value of the called function? A quick grep revealed that there is plenty of code that ignores the return value of these last two functions. Thanks, Bart.