by Myklebust, Trond

[permalink] [raw]

Subject: Re: return layout on error, BUG/deadlock

T24gVGh1LCAyMDEyLTA4LTA5IGF0IDE4OjQ5ICswMzAwLCBJZGFuIEtlZGFyIHdyb3RlOg0KPiBP
biBUaHUsIEF1ZyA5LCAyMDEyIGF0IDU6MDUgUE0sIE15a2xlYnVzdCwgVHJvbmQNCj4gPFRyb25k
Lk15a2xlYnVzdEBuZXRhcHAuY29tPiB3cm90ZToNCj4gPj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdl
LS0tLS0NCj4gPj4gRnJvbTogbGludXgtbmZzLW93bmVyQHZnZXIua2VybmVsLm9yZyBbbWFpbHRv
OmxpbnV4LW5mcy0NCj4gPj4gb3duZXJAdmdlci5rZXJuZWwub3JnXSBPbiBCZWhhbGYgT2YgSWRh
biBLZWRhcg0KPiA+PiBTZW50OiBUaHVyc2RheSwgQXVndXN0IDA5LCAyMDEyIDk6MDMgQU0NCj4g
Pj4gVG86IEJvYXogSGFycm9zaDsgTkZTIGxpc3QNCj4gPj4gQ2M6IEJlbm55IEhhbGV2eQ0KPiA+
PiBTdWJqZWN0OiByZXR1cm4gbGF5b3V0IG9uIGVycm9yLCBCVUcvZGVhZGxvY2sNCj4gPj4NCj4g
Pj4gSGksDQo+ID4+DQo+ID4+IEFzIGEgcmVzdWx0IG9mIHNvbWUgZXhwZXJpbWVudHMsIEkgd2Fu
dGVkIHRvIHNlZSB3aGF0IGhhcHBlbnMgd2hlbiBJDQo+ID4+IGluamVjdCBhbiBlcnJvciAoaGFy
ZCBjb2RlZCkgdG8gdGhlIG9iamVjdCBsYXlvdXQgZHJpdmVyLiB0aGUgcGF0Y2ggaXMgYXQgdGhl
DQo+ID4+IGJvdHRvbSBvZiB0aGlzIG1haWwuIHRoZSByZWFzb24gSSBkaWQgdGhpcyBpcyBiZWNh
dXNlIHdoZW4gSSBpbmplY3QgZXJyb3JzIGluIG15DQo+ID4+IG1vZGlmaWVkIHZlcnNpb24gb2Yg
dGhlIG9iamVjdCBsYXlvdXQgZHJpdmVyLCBJIGdldCB0aGUgc2FtZSBCVUcgVGlncmFuDQo+ID4+
IHJlcG9ydGVkIGFib3V0IHllc3RlcmRheToNCj4gPj4gbmZzNHByb2MuYzo2MjUyIDogICBCVUdf
T04oIWxpc3RfZW1wdHkoJmxvLT5wbGhfc2VncykpOw0KPiA+Pg0KPiA+PiBJbiBteSBtb2RpZmll
ZCB2ZXJzaW9uIChiYXNlZCBvbiBrZXJuZWwgMy4zKSwgdGhlIGJ1ZyBzZWVtcyB0byBiZSB0aGF0
DQo+ID4+IHBuZnNfbGRfd3JpdGVfZG9uZSBjYWxscyBwbmZzX3JldHVybl9sYXlvdXQgaW4gdGhl
IGVycm9yIHBhdGgsIGV2ZW4gaWYgdGhlcmUNCj4gPj4gaXMgaW4tZmxpZ2h0IEkvTy4NCj4gPg0K
PiA+IFRoYXQgaXMgbm90IGEgYnVnLiBJdCBpcyBhbiBpbnRlbnRpb25hbCBjaGFuZ2UgaW4gb3Jk
ZXIgdG8gYWxsb3cgdGhlIE1EUyB0byBmZW5jZSBvZmYgdGhlIG91dHN0YW5kaW5nIHdyaXRlcyAo
aWYgaXQgY2FuIGRvIHNvKSBiZWZvcmUgd2UgcmV0cmFuc21pdCB0aGVtIGFzIHdyaXRlLXRocm91
Z2gtTURTLiBPdGhlcndpc2UsIHlvdSByaXNrIHJhY2VzIGJldHdlZW4gdGhlIG91dHN0YW5kaW5n
IHdyaXRlcy10by1EUyBhbmQgdGhlIG5ldyB3cml0ZXMtdGhyb3VnaC1NRFMuDQo+IA0KPiB0byB3
aGF0IGNoYW5nZSBhcmUgeW91IHJlZmVycmluZz8NCg0KQXMgSSBzdGF0ZWQgaW4gdGhlIGNoYW5n
ZWxvZyBvZiB0aGUgcGF0Y2ggdGhhdCBJIHNlbnQgdG8gdGhlIGxpc3QNCnllc3RlcmRheSwgdGhl
IGJlaGF2aW91ciBpcyBkdWUgdG8gY29tbWl0IDBhNTdjZGFjM2YuDQoNCj4gPg0KPiA+IFNlZSB0
aGUgY2hhbmdlbG9nIGluIHRoZSBwYXRjaCB0aGF0IEkgc2VudCB0byB0aGUgbGlzdCB5ZXN0ZXJk
YXkuDQo+ID4NCj4gDQo+IEkgc2F3IHRoYXQsIGFuZCBpZiBJJ20gbm90IG1pc3Rha2VuIHRoZXNl
IHJhY2VzIGFwcGx5IHRvIG9iamVjdCBsYXlvdXQNCj4gYXMgd2VsbCwgYW5kIGluIGFueSBjYXNl
IHRoZXkgYXBwbHkgaW4gbXkgY2FzZS4gSG93ZXZlciwgaXQgaXMgbm90DQo+IGVhc3kgdG8gbWVz
cyBhcm91bmQgd2l0aCBMQVlPVVRSRVRVUk4gaW4gb2JqZWN0IGxheW91dCwgYW5kIHRoZXJlIGhh
dmUNCj4gYmVlbiBzZXZlcmFsIGRpc2N1c3Npb25zIG9uIHRoZSBpc3N1ZS4gSW4gb25lIG9mIHRo
ZXNlIGRpc2N1c3Npb25zDQo+IEJlbm55IGNsYXJpZmllZCB0aGF0IHRoZSBvYmplY3QgbGF5b3V0
IGNsaWVudCBtdXN0IHdhaXQgZm9yIGFsbA0KPiBpbi1mbGlnaHQgSS9PIHRvIGVuZC4NCg0KSWYg
dGhlIHByb2JsZW0gaXMgdGhhdCB0aGUgRFMgaXMgZmFpbGluZyB0byByZXNwb25kLCBob3cgZG9l
cyB0aGUgY2xpZW50DQprbm93IHRoYXQgdGhlIGluLWZsaWdodCBJL08gaGFzIGVuZGVkPw0KDQo+
IFNvIGZvciBmaWxlIGxheW91dCBpdCBwcm9iYWJseSBtYWtlcyBzZW5zZSwgYnV0IG9iamVjdCBs
YXlvdXQgKGFuZCBpZg0KPiBJIHVuZGVyc3RhbmQgY29ycmVjdGx5LCBibG9jayBsYXlvdXQgYXMg
d2VsbCkgc29tZXRoaW5nIGVsc2UgbmVlZHMgdG8NCj4gYmUgZG9uZS4gSSB0aG91Z2h0IGFib3V0
IHN5bmMgd2FpdCB3aGVuIHJldHVybmluZyB0aGUgbGF5b3V0IG9uIGVycm9yLA0KPiBidXQgYWNj
b3JkaW5nIHRvIEJvYXogaXQgd2lsbCBjYXVzZSBkZWFkbG9ja3MgKEJvYXogLSBjYW4geW91IHBs
ZWFzZQ0KPiBlbGFib3JhdGU/KS4NCg0KVGhlIG9iamVjdCBsYXlvdXRyZXR1cm4gaGFzIHRoZSBh
YmlsaXR5IHRvIHBhc3MgYSB0aW1lb3V0IGVycm9yIHZhbHVlIHRvDQp0aGUgTURTIHByZWNpc2Vs
eSBpbiBvcmRlciB0byBhbGxvdyB0aGUgbGF0dGVyIHRvIGRlYWwgd2l0aCB0aGlzIGtpbmQgb2YN
Cmlzc3VlLiBTZWUgdGhlIGRlc2NyaXB0aW9uIG9mIHN0cnVjdCBwbmZzX29zZF9pb2VycjQgaW4g
cmZjNTY2NC4NCg0KVGhlIGJsb2NrIGxheW91dCBpcyBhZGRpbmcgdGhlIHNhbWUgYWJpbGl0eSB0
byBsYXlvdXRyZXR1cm4gaW4gTkZTdjQuMg0KKHNlZSBkcmFmdC1pZXRmLW5mc3Y0LW1pbm9ydmVy
c2lvbjItMTMudHh0KSB2aWEgdGhlIHN0cnVjdA0KbGF5b3V0cmV0dXJuX2RldmljZV9lcnJvcjQs
IHNvIHByZXN1bWFibHkgdGhleSB0b28gaGF2ZSBhIHBsYW4gZm9yDQpkZWFsaW5nIHdpdGggdGhp
cyBraW5kIG9mIGlzc3VlLg0KDQo+IEFuZCBjb21lIHRvIHRoaW5rIG9mIGl0LCBuZnM0X3Byb2Nf
c2V0YXR0ciBhbHNvIHJldHVybnMgdGhlIGxheW91dA0KPiB3aGVuIHRoZXJlIG1heSBiZSBJL08g
aW4tZmxpZ2h0IChjb3JyZWN0IG1lIGlmIGknbSB3cm9uZykuIFNvIEkgZ3Vlc3MNCj4gcG5mc19y
ZXR1cm5fbGF5b3V0IHNob3VsZCBzb21laG93IHNvbHZlIHRoaXMgYnkgaXRzZWxmIGJ5IHNheWlu
ZyAiaWYNCj4gdGhpcyBpcyBmZW5jaW5nIChhIGZsYWcgd2hpY2ggd2lsbCBiZSBzZXQgZm9yIGZp
bGUgbGF5b3V0IG9ubHkpLCBnbw0KPiBhaGVhZCwgb3RoZXJ3aXNlIG1ha2UgdGhlIGxheW91dCBh
cyAnbmVlZHMgdG8gYmUgcmV0dXJuZWQnIGFuZCB3aGVuDQo+IHRoZSBsc2VnIGxpc3RzIGdldHMg
ZW1wdHkgcmV0dXJuIHRoZSBsYXlvdXQiLg0KDQpUaGUgb25seSBsYXlvdXQgdHlwZSB0aGF0IHNl
dHMgdGhlIFBORlNfTEFZT1VUUkVUX09OX1NFVEFUVFIgZmxhZyBpcw0Kb2JqZWN0cywgc28gdGhh
dCBxdWVzdGlvbiBuZWVkcyB0byBiZSBkaXJlY3RlZCB0byBCb2F6Lg0KDQotLSANClRyb25kIE15
a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlr
bGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

2012-08-10 02:47:09

by Peng Tao

[permalink] [raw]

Subject: Re: return layout on error, BUG/deadlock

On Fri, Aug 10, 2012 at 3:31 AM, Myklebust, Trond
<[email protected]> wrote:
> On Fri, 2012-08-10 at 00:48 +0800, Peng Tao wrote:
>> On Fri, Aug 10, 2012 at 12:37 AM, Myklebust, Trond
>> <[email protected]> wrote:
>> > On Fri, 2012-08-10 at 00:34 +0800, Peng Tao wrote:
>> >> On Fri, Aug 10, 2012 at 12:06 AM, Myklebust, Trond
>> >> <[email protected]> wrote:
>> >> > On Thu, 2012-08-09 at 18:49 +0300, Idan Kedar wrote:
>> >> >> On Thu, Aug 9, 2012 at 5:05 PM, Myklebust, Trond
>> >> >> <[email protected]> wrote:
>> >> >> >> -----Original Message-----
>> >> >> >> From: [email protected] [mailto:linux-nfs-
>> >> >> >> [email protected]] On Behalf Of Idan Kedar
>> >> >> >> Sent: Thursday, August 09, 2012 9:03 AM
>> >> >> >> To: Boaz Harrosh; NFS list
>> >> >> >> Cc: Benny Halevy
>> >> >> >> Subject: return layout on error, BUG/deadlock
>> >> >> >>
>> >> >> >> Hi,
>> >> >> >>
>> >> >> >> As a result of some experiments, I wanted to see what happens when I
>> >> >> >> inject an error (hard coded) to the object layout driver. the patch is at the
>> >> >> >> bottom of this mail. the reason I did this is because when I inject errors in my
>> >> >> >> modified version of the object layout driver, I get the same BUG Tigran
>> >> >> >> reported about yesterday:
>> >> >> >> nfs4proc.c:6252 : BUG_ON(!list_empty(&lo->plh_segs));
>> >> >> >>
>> >> >> >> In my modified version (based on kernel 3.3), the bug seems to be that
>> >> >> >> pnfs_ld_write_done calls pnfs_return_layout in the error path, even if there
>> >> >> >> is in-flight I/O.
>> >> >> >
>> >> >> > That is not a bug. It is an intentional change in order to allow the MDS to fence off the outstanding writes (if it can do so) before we retransmit them as write-through-MDS. Otherwise, you risk races between the outstanding writes-to-DS and the new writes-through-MDS.
>> >> >>
>> >> >> to what change are you referring?
>> >> >
>> >> > As I stated in the changelog of the patch that I sent to the list
>> >> > yesterday, the behaviour is due to commit 0a57cdac3f.
>> >> >
>> >> >> >
>> >> >> > See the changelog in the patch that I sent to the list yesterday.
>> >> >> >
>> >> >>
>> >> >> I saw that, and if I'm not mistaken these races apply to object layout
>> >> >> as well, and in any case they apply in my case. However, it is not
>> >> >> easy to mess around with LAYOUTRETURN in object layout, and there have
>> >> >> been several discussions on the issue. In one of these discussions
>> >> >> Benny clarified that the object layout client must wait for all
>> >> >> in-flight I/O to end.
>> >> >
>> >> > If the problem is that the DS is failing to respond, how does the client
>> >> > know that the in-flight I/O has ended?
>> >> >
>> >> >> So for file layout it probably makes sense, but object layout (and if
>> >> >> I understand correctly, block layout as well) something else needs to
>> >> >> be done. I thought about sync wait when returning the layout on error,
>> >> >> but according to Boaz it will cause deadlocks (Boaz - can you please
>> >> >> elaborate?).
>> >> >
>> >> > The object layoutreturn has the ability to pass a timeout error value to
>> >> > the MDS precisely in order to allow the latter to deal with this kind of
>> >> > issue. See the description of struct pnfs_osd_ioerr4 in rfc5664.
>> >> >
>> >> > The block layout is adding the same ability to layoutreturn in NFSv4.2
>> >> > (see draft-ietf-nfsv4-minorversion2-13.txt) via the struct
>> >> > layoutreturn_device_error4, so presumably they too have a plan for
>> >> > dealing with this kind of issue.
>> >> It is one thing to tell MDS that there is DS access error by sending
>> >> layoutreturn, and it is another thing to return a layout even if there
>> >> is overlapping in-flight DS IO...
>> >>
>> >> I certainly agree that client is entitled to return layout to inform
>> >> MDS about DS errors and also avoid possible cb_layoutrecall. But it is
>> >> just an optimization and should only be done when there is no
>> >> in-flight IO (at least for block layout) IMHO.
>> >
>> > HOW DO YOU GUARANTEE NO IN-FLIGHT IO?
>> >
>> I don't. That's why I don't return layout in pnfs_ld_write_done(). And
>> for layoutreturn upon cb_layoutreturn, block layout client needs to do
>> timed-lease IO fencing per rfc5663, but it is not implemented in Linux
>> client.
>
> The timed-lease IO fencing described in rfc5663 is about informing the
> server about how long the client expects a command to succeed or fail.
> It doesn't offer any advice for how the client is to deal with an
> unresponsive DS.
>
> What you need here is help from the underlying transport protocol. As I
> said in the email to Idan, when researching iSCSI and iFCP, I found what
> appears to be mechanisms for reliably timing out.
Just checked and found that the layoutreturn-on-error behavior only
affects object and file layout. So block layout stays out and safe.
That's all I would ask for. Thanks for your explanation.

Best,
Tao