kernel BUG at /build/buildd/linux-3.2.0/fs/lockd/clntxdr.c:226!

Guys, check this report from Larry out.

Also, why the *HELL* is that a BUG_ON() in the first place? Who was the less-than-gifted person who decided "if this thing can happen, let's just kill the whole machine"?

BUG_ON()'s are for machine-killing problems, not for some random assert() in the code. If the thing can never happen, then the BUG_ON() shouldn't have existed in the first place. And if you are worried about it happening (like it clearly did for Larry), ou should have handled it, since clearly this is *not* a machine-killing worthy problem.

IOW, something like

        WARN_ON_ONCE(be32_to_cpu(stat) > NLM_LCK_DENIED_GRACE_PERIOD);

would have been way more appropriate. Let people know that there is a problem, but don't kill the machine.

We have way too many people who seem to think that "I don't know what I should do, so I'll just kill the machine" is a sane option. It's not. Sure, often a BUG_ON() is survivable (just killing the process), but in filesystem code there's usually a lock (or many) that tends to make it problematic even when we can just kill the process, and often causes these things to not even be logged very well.

Larry, the stack trace and registers would be useful. Picture or a full dump of the BUG_ON() if it got logged? If it gets eaten by the machine being unresponsive after the event and since you can reproduce it, you could just try to change it to the WARN_ON_ONCE() above, and then it should be easier to just get out of the dmesg, since hopefully the machine stays up despite the odd status value..

Linus

Ahh, I've been away from the kernel too long. I miss that delicate management touch.

Been a while since I've built a kernel and this is our production file server, it goes down and our whole company stops. As surprising as it might sound, given git's success, we're still busy so crashing the server isn't fun :)

pics of the stack trace at http://www.mcvoy.com/lm/nfs-lock-crash

---
Larry McVoy                lm at bitmover.com           http://www.bitkeeper.com

"Delicate Management Touch" is my middle name.

Ok, that's just the normal kind of random left-over oopses due to subsequent problems of a BUG_ON(). Looks like the watchdog timer ends up being unhappy, almost certainly simply because some core filesystem spinlock not being released.

It used to be (a long long time ago) that we'd recover fairly gracefully from BUG_ON()'s - back when the main shared lock we had was the kernel lock, and we had a single per-process kernel lock counter. So when we killed the process, we could clean that single lock up.

These days, if some process dies in random kernel code due to a BUG_ON() or a wild pointer or similar, and we kill it, we are seldom able to do so cleanly. So the best we can hope for is that it happened in some context where it held no (important) locks. Which is rare. So BUG_ON()'s are often fatal, and there are these kinds of downstream problems where they get flushed off the screen by subsequent issues...

Ho humm. Google doesn't seem to be finding any similar bug-reports, so unless Bruce or Trond go "Ahh, I know what it's about", I do think we would want to get as much more info as possible.

Doing a kernel compile really isn't that bad. The only nasty piece is getting the kernel configuration right, but you can just use the distro config. It's much too big and contains everything, but it will work, and gets you as similar a kernel as possible. Of course, Ubuntu has made installing your own kernel stupidly complicated (you have to build a package and install it using the package manager), but while it's an annoying extra step or two (compared to just doing a "make modules_install install"), it's not rocket surgery. There's a few help pages for it:

https://help.ubuntu.com/community/Kernel/Compile

being the first one.

Linus

Linus Torvalds wrote:

Doing a kernel compile really isn't that bad. The only nasty piece is getting the kernel configuration right, but you can just use the distro config. It's much too big and contains everything, but it will work, and gets you as similar a kernel as possible. Of course, Ubuntu has made installing your own kernel stupidly complicated (you have to build a package and install it using the package manager), but while it's an annoying extra step or two (compared to just doing a "make modules_install install"), it's not rocket surgery.

I install kernels on Ubuntu every day of the week and twice on Sundays, and never jump through their silly hoops. The only magic is making the initramfs, if you use one.

make install modules_install
update-initramfs -c -k 3.6.1 (or whatever)
update-grub

This magic is what the distro "/usr/sbin/installkernel" script is supposed to do, which the kernel Makefile does for the "install" target.

So the "make install" *should* have done the grub and initramfs update. Ubuntu screwed up (probably because Debian did).

I wish they'd just fix it. The whole reason the kernel does that /usr/sbin/installkernel thing is so that distributions can do whatever magic it is they want to do for themselves.

Linus

I thought "make install" is suppose to call a distro specific driver script to do exactly that initramfs thing.

Some good sole should just fix it up on Broken ubuntu. I would but I don't use ubuntu I use Fedora.

Just "which installkernel" usually /sbin/installkernel and fix in there, I think.

And that too, it's a new grub2 thing, right

Cheers
Boaz

Something is trivially weird

fs/lockd/clntxdr.c:226 is in this static function:
encode_nlm_stat()

encode_nlm_stat() is called in two places
static void nlm_xdr_enc_res(...)
and
static void nlm_xdr_enc_testres(...)

But these two are not called anywhere. In-fact a Kernel wide grep returns a single occurrence of both

I suspect the BUG_ON() belongs else where.
Perhaps we should add __func__ to the BUG_ON macro exactly for times that source and Kernel get out of sync.

Puzzled
Boaz

T24gU2F0LCAyMDEyLTEwLTEzIGF0IDEwOjAyICswOTAwLCBMaW51cyBUb3J2YWxkcyB3cm90ZToN Cj4gT24gU2F0LCBPY3QgMTMsIDIwMTIgYXQgOToyMSBBTSwgTGFycnkgTWNWb3kgPGxtQGJpdG1v dmVyLmNvbT4gd3JvdGU6DQo+ID4NCj4gPiBBaGgsIEkndmUgYmVlbiBhd2F5IGZyb20gdGhlIGtl cm5lbCB0b28gbG9uZy4gIEkgbWlzcyB0aGF0IGRlbGljYXRlDQo+ID4gbWFuYWdlbWVudCB0b3Vj aC4NCj4gDQo+ICJEZWxpY2F0ZSBNYW5hZ2VtZW50IFRvdWNoIiBpcyBteSBtaWRkbGUgbmFtZS4N Cj4gDQo+ID4gcGljcyBvZiB0aGUgc3RhY2sgdHJhY2UgYXQgaHR0cDovL3d3dy5tY3ZveS5jb20v bG0vbmZzLWxvY2stY3Jhc2gNCj4gDQo+IE9rLCB0aGF0J3MganVzdCB0aGUgbm9ybWFsIGtpbmQg b2YgcmFuZG9tIGxlZnQtb3ZlciBvb3BzZXMgZHVlIHRvDQo+IHN1YnNlcXVlbnQgcHJvYmxlbXMg b2YgYSBCVUdfT04oKS4gTG9va3MgbGlrZSB0aGUgd2F0Y2hkb2cgdGltZXIgZW5kcw0KPiB1cCBi ZWluZyB1bmhhcHB5LCBhbG1vc3QgY2VydGFpbmx5IHNpbXBseSBiZWNhdXNlIHNvbWUgY29yZSBm aWxlc3lzdGVtDQo+IHNwaW5sb2NrIG5vdCBiZWluZyByZWxlYXNlZC4NCj4gDQo+IEl0IHVzZWQg dG8gYmUgKGEgbG9uZyBsb25nIHRpbWUgYWdvKSB0aGF0IHdlJ2QgcmVjb3ZlciBmYWlybHkNCj4g Z3JhY2VmdWxseSBmcm9tIEJVR19PTigpJ3MgLSBiYWNrIHdoZW4gdGhlIG1haW4gc2hhcmVkIGxv Y2sgd2UgaGFkIHdhcw0KPiB0aGUga2VybmVsIGxvY2ssIGFuZCB3ZSBoYWQgYSBzaW5nbGUgcGVy LXByb2Nlc3Mga2VybmVsIGxvY2sgY291bnRlci4NCj4gU28gd2hlbiB3ZSBraWxsZWQgdGhlIHBy b2Nlc3MsIHdlIGNvdWxkIGNsZWFuIHRoYXQgc2luZ2xlIGxvY2sgdXAuDQo+IA0KPiBUaGVzZSBk YXlzLCBpZiBzb21lIHByb2Nlc3MgZGllcyBpbiByYW5kb20ga2VybmVsIGNvZGUgZHVlIHRvIGEN Cj4gQlVHX09OKCkgb3IgYSB3aWxkIHBvaW50ZXIgb3Igc2ltaWxhciwgYW5kIHdlIGtpbGwgaXQs IHdlIGFyZSBzZWxkb20NCj4gYWJsZSB0byBkbyBzbyBjbGVhbmx5LiBTbyB0aGUgYmVzdCB3ZSBj YW4gaG9wZSBmb3IgaXMgdGhhdCBpdCBoYXBwZW5lZA0KPiBpbiBzb21lIGNvbnRleHQgd2hlcmUg aXQgaGVsZCBubyAoaW1wb3J0YW50KSBsb2Nrcy4gV2hpY2ggaXMgcmFyZS4gU28NCj4gQlVHX09O KCkncyBhcmUgb2Z0ZW4gZmF0YWwsIGFuZCB0aGVyZSBhcmUgdGhlc2Uga2luZHMgb2YgZG93bnN0 cmVhbQ0KPiBwcm9ibGVtcyB3aGVyZSB0aGV5IGdldCBmbHVzaGVkIG9mZiB0aGUgc2NyZWVuIGJ5 IHN1YnNlcXVlbnQgaXNzdWVzLi4uDQoNCklmIHRoYXQgY29kZSBpcyBiZWluZyBjYWxsZWQgdW5k ZXIgYSBsb2NrLCB0aGVuIHdlIGhhdmUgb3RoZXIgcHJvYmxlbXMuDQpJdCBpcyBzdGFuZGFyZCBY RFIgY29kZTogaXQgc2hvdWxkIGFsd2F5cyBiZSBjYWxsZWQgZnJvbSBhbiBvcmRpbmFyeQ0KcHJv Y2VzcyBjb250ZXh0IHdpdGggbm8gc3BlY2lhbCBsb2NrcyBiZWluZyBoZWxkIGJ5IHRoZSBjYWxs ZXJzLg0KDQo+IEhvIGh1bW0uIEdvb2dsZSBkb2Vzbid0IHNlZW0gdG8gYmUgZmluZGluZyBhbnkg c2ltaWxhciBidWctcmVwb3J0cywgc28NCj4gdW5sZXNzIEJydWNlIG9yIFRyb25kIGdvICJBaGgs IEkga25vdyB3aGF0IGl0J3MgYWJvdXQiLCBJIGRvIHRoaW5rIHdlDQo+IHdvdWxkIHdhbnQgdG8g Z2V0IGFzIG11Y2ggbW9yZSBpbmZvIGFzIHBvc3NpYmxlLg0KDQpOZXZlciBzZWVuIGl0IGJlZm9y ZSwgYW5kIEkgc2VlIG5vIHJlYXNvbiB3aHkgaXQgc2hvdWxkIGRyYWcgdGhlIGVudGlyZQ0KYm94 IGRvd24gd2l0aCBpdC4gSXQgaXMgcGFydCBvZiB0aGUgTkxNIHNlcnZlcidzIGNhbGxiYWNrIGNv ZGUsIHNvIHRoZXJlDQppcyBubyBjaGFuY2Ugb2YgaXQgYmVpbmcgY2FsbGVkIGFzIHBhcnQgb2Yg YSBtZW1vcnkgcmVjbGFpbSBvciBhbnl0aGluZw0Kc2ltaWxhcmx5IHNlbnNpdGl2ZSB0byB0aGUg cmVzdCBvZiB0aGUgYm94Lg0KDQpBcmUgd2Ugc3VyZSB0aGF0IHRoaXMgQlVHX09OKCkgcmVhbGx5 IGlzIHRvcCBvZiB0aGUgY2hhaW4gb2YgT29wc2VzDQpoZXJlPyBBbGwgSSBjYW4gc2VlIGl0IGRv aW5nIGlzIGNyYXNoaW5nIHRoZSBsb2NrZCBzZXJ2ZXIgcHJvY2Vzcywgd2hpY2gNCndpbGwgc2Vy aW91c2x5IGluY29udmVuaWVuY2UgYWxsIHRoZSBORlMgY2xpZW50cyB0cnlpbmcgdG8gZG8gbG9j a2luZywNCmJ1dCBpdCBzaG91bGRuJ3QgYmUgYWZmZWN0aW5nIHRoZSBzd2FwcGVyIHByb2Nlc3Mg YXMgd2UncmUgc2VlaW5nIGluIHRoZQ0KT29wcyBzY3JlZW5zaG90cy4NCklmIGl0IHJlYWxseSBp cyB0aGUgZmlyc3QgdGhpbmcgdG8gT29wcywgdGhlbiB0aGUgb25seSB0aGluZyBJIGNhbiB0aGlu aw0Kb2YgdGhlcmUgdGhhdCB3b3VsZCB0cmlnZ2VyIG90aGVyIE9vcHNlcyB3b3VsZCBiZSBhIG1l bW9yeSBjb3JydXB0aW9uDQoodXNlIGFmdGVyIGZyZWUgb3Igc29tZSBzdWNoIHRoaW5nPykuIFBl cmhhcHMgTGFycnkgY291bGQgdHJ5IHR1cm5pbmcgb24NCnNvbWUgb2YgdGhlIGxlc3MgaW50cnVz aXZlIHNsYWIgZGVidWdnaW5nIG9wdGlvbnM/DQoNCj4gRG9pbmcgYSBrZXJuZWwgY29tcGlsZSBy ZWFsbHkgaXNuJ3QgdGhhdCBiYWQuIFRoZSBvbmx5IG5hc3R5IHBpZWNlIGlzDQo+IGdldHRpbmcg dGhlIGtlcm5lbCBjb25maWd1cmF0aW9uIHJpZ2h0LCBidXQgeW91IGNhbiBqdXN0IHVzZSB0aGUN Cj4gZGlzdHJvIGNvbmZpZy4gSXQncyBtdWNoIHRvbyBiaWcgYW5kIGNvbnRhaW5zIGV2ZXJ5dGhp bmcsIGJ1dCBpdCB3aWxsDQo+IHdvcmssIGFuZCBnZXRzIHlvdSBhcyBzaW1pbGFyIGEga2VybmVs IGFzIHBvc3NpYmxlLiBPZiBjb3Vyc2UsIFVidW50dQ0KPiBoYXMgbWFkZSBpbnN0YWxsaW5nIHlv dXIgb3duIGtlcm5lbCBzdHVwaWRseSBjb21wbGljYXRlZCAoeW91IGhhdmUgdG8NCj4gYnVpbGQg YSBwYWNrYWdlIGFuZCBpbnN0YWxsIGl0IHVzaW5nIHRoZSBwYWNrYWdlIG1hbmFnZXIpLCBidXQg d2hpbGUNCj4gaXQncyBhbiBhbm5veWluZyBleHRyYSBzdGVwIG9yIHR3byAoY29tcGFyZWQgdG8g anVzdCBkb2luZyBhICJtYWtlDQo+IG1vZHVsZXNfaW5zdGFsbCBpbnN0YWxsIiksIGl0J3Mgbm90 IHJvY2tldCBzdXJnZXJ5LiBUaGVyZSdzIGEgZmV3IGhlbHANCj4gcGFnZXMgZm9yIGl0Og0KPiAN Cj4gICAgIGh0dHBzOi8vaGVscC51YnVudHUuY29tL2NvbW11bml0eS9LZXJuZWwvQ29tcGlsZQ0K PiANCj4gYmVpbmcgdGhlIGZpcnN0IG9uZS4NCj4gDQo+ICAgICAgICAgICAgICAgICBMaW51cw0K DQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5l dEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQo=

In BitKeeper, we wacked perror() to print out file, line. Saved us a ton of debugging. Oh, and bk-version as well. That helps.

---
Larry McVoy                lm at bitmover.com           http://www.bitkeeper.com

As I've said, debugging this is going to be hard, it means stopping my company. We can do one more crash, you guys can tell me what to do, but that's about it.

---
Larry McVoy                lm at bitmover.com           http://www.bitkeeper.com

T24gRnJpLCAyMDEyLTEwLTEyIGF0IDE5OjI3IC0wNzAwLCBCb2F6IEhhcnJvc2ggd3JvdGU6DQo+ IE9uIDEwLzEyLzIwMTIgMDQ6NTIgUE0sIExpbnVzIFRvcnZhbGRzIHdyb3RlOg0KPiA+IEd1eXMs IGNoZWNrIHRoaXMgcmVwb3J0IGZyb20gTGFycnkgb3V0Lg0KPiA+IA0KPiA+IEFsc28sIHdoeSB0 aGUgKkhFTEwqIGlzIHRoYXQgYSBCVUdfT04oKSBpbiB0aGUgZmlyc3QgcGxhY2U/IFdobyB3YXMN Cj4gPiB0aGUgbGVzcy10aGFuLWdpZnRlZCBwZXJzb24gd2hvIGRlY2lkZWQgImlmIHRoaXMgdGhp bmcgY2FuIGhhcHBlbiwNCj4gPiBsZXQncyBqdXN0IGtpbGwgdGhlIHdob2xlIG1hY2hpbmUiPw0K PiA+IA0KPiANCj4gU29tZXRoaW5nIGlzIHRyaXZpYWxseSB3ZWlyZA0KPiANCj4gZnMvbG9ja2Qv Y2xudHhkci5jOjIyNiBpcyBpbiB0aGlzIHN0YXRpYyBmdW5jdGlvbjoNCj4gCWVuY29kZV9ubG1f c3RhdCgpDQo+IA0KPiBlbmNvZGVfbmxtX3N0YXQoKSBpcyBjYWxsZWQgaW4gdHdvIHBsYWNlcw0K PiAJc3RhdGljIHZvaWQgbmxtX3hkcl9lbmNfcmVzKC4uLikNCj4gICBhbmQNCj4gCXN0YXRpYyB2 b2lkIG5sbV94ZHJfZW5jX3Rlc3RyZXMoLi4uKQ0KPiANCj4gQnV0IHRoZXNlIHR3byBhcmUgbm90 IGNhbGxlZCBhbnl3aGVyZS4gSW4tZmFjdCBhIEtlcm5lbCB3aWRlIGdyZXANCj4gcmV0dXJucyBh IHNpbmdsZSBvY2N1cnJlbmNlIG9mIGJvdGgNCg0KVGhleSBhcmUgdXNlZCBpbiB0aGUgbmxtX3By b2NlZHVyZXNbXSBhcnJheSBhcyBURVNUX1JFUywgTE9DS19SRVMsDQpDQU5DRUxfUkVTLCBVTkxP Q0tfUkVTIGFuZCBHUkFOVEVEX1JFUyB4ZHIgZW5jb2RpbmcgcHJvY2VkdXJlcy4NCg0KDQoNCi0t IA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBw DQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg==

BUG_ON already does the whole "line and file" thing, so the exact BUG_ON() should be very well pinpointed.

Sometimes various distro changes do make the lines move around, but clntxdr.c has been very stable, and line 226 has the same BUG_ON in it in both 3.2 and 3.6, so I seriously doubt that's the case.

And that function is definitely called, it's just that you missed the magic cpp stuff. See the preprocessor using it like this:

.p_encode = (kxdreproc_t)nlm_xdr_enc_##argtype,

so the nlm_xdr_enc_res() function ends up being used as the .p_encode function for the 'res' argtype.. (and same for testres)

Linus

Ha, sorry that was impossible to find. OK I'll have a look

Thanks
Boaz

I could go in on a weekend and bang on things if that helps. Here's some other info: we use old systems so we are forward compat. The mac was / is running MacOS 10.4:

macos-x86:~ uname -a
Darwin macos-x86.bitmover.com 8.11.1 Darwin Kernel Version 8.11.1: Wed Oct 10 18:23:28 PDT 2007; root:xnu-792.25.20~1/RELEASE_I386 i386 i386

No idea if they fucked up NFS, certainly possible.

The way to make it happen is to have your home directory mounted on that machine and fire up skype. It tries to do some locking thing and barfs.

How do I figure out which NFS version macos is using?

macos-x86:~ mount
/dev/disk0s2 on / (local, journaled)
devfs on /dev (local)
fdesc on /dev (union)
<volfs> on /.vol
automount -nsl [144] on /Network (automounted) automount -fstab [148] on /automount/Servers (automounted) automount -static [148] on /automount/static (automounted) 10.3.9.1:/home on /private/var/automount/home (automounted)

---
Larry McVoy                lm at bitmover.com           http://www.bitkeeper.com

T24gRnJpLCAyMDEyLTEwLTEyIGF0IDE5OjMxIC0wNzAwLCBMYXJyeSBNY1ZveSB3cm90ZToNCj4g QXMgSSd2ZSBzYWlkLCBkZWJ1Z2dpbmcgdGhpcyBpcyBnb2luZyB0byBiZSBoYXJkLCBpdCBtZWFu cyBzdG9wcGluZyBteQ0KPiBjb21wYW55LiAgV2UgY2FuIGRvIG9uZSBtb3JlIGNyYXNoLCB5b3Ug Z3V5cyBjYW4gdGVsbCBtZSB3aGF0IHRvIGRvLA0KPiBidXQgdGhhdCdzIGFib3V0IGl0Lg0KDQpE byB5b3UgaGF2ZSBhIHJlcHJvZHVjZXI/IElmIHlvdSBkbywgdGhlbiBwZXJoYXBzIHdlIGNhbiB0 cnkgdG8NCmR1cGxpY2F0ZSB0aGUgaXNzdWUgb24gb3VyIG93biBzZXR1cHMgd2l0aG91dCBuZWVk aW5nIHRvIGRlbGliZXJhdGVseQ0KY2F1c2UgeW91IG1vcmUgZG93bnRpbWUuDQoNCklmIG5vdCwg dGhlbiBJIGd1ZXNzIHdlJ2xsIGhhdmUgdG8gdHJ5IGxvb2tpbmcgYXQgdGhvc2Ugc2xhYiBtZW1v cnkNCmRlYnVnZ2luZyBvcHRpb25zLiBJJ20gbm8gZXhwZXJ0IG9uIHNsYWIgZGVidWdnaW5nLCBh bmQgaXQncyB3YXkgdG9vDQpsYXRlIGluIHRoaXMgY29ybmVyIG9mIHRoZSBVUyBmb3IgbWUgdG8g dHJ5IHRvIGZpZ3VyZSBvdXQgZXhhY3RseSB3aGF0DQpvcHRpb25zIGFyZSBhcHByb3ByaWF0ZSwg YnV0IEkgY2FuIGxvb2sgaW50byBpdCB0b21vcnJvdyAodW5sZXNzIExpbnVzDQpoYXMgYSBzdWdn ZXN0aW9uKS4NCg0KPiBPbiBTYXQsIE9jdCAxMywgMjAxMiBhdCAwMjoyODozOUFNICswMDAwLCBN eWtsZWJ1c3QsIFRyb25kIHdyb3RlOg0KPiA+IE9uIFNhdCwgMjAxMi0xMC0xMyBhdCAxMDowMiAr MDkwMCwgTGludXMgVG9ydmFsZHMgd3JvdGU6DQo+ID4gPiBPbiBTYXQsIE9jdCAxMywgMjAxMiBh dCA5OjIxIEFNLCBMYXJyeSBNY1ZveSA8bG1AYml0bW92ZXIuY29tPiB3cm90ZToNCj4gPiA+ID4N Cj4gPiA+ID4gQWhoLCBJJ3ZlIGJlZW4gYXdheSBmcm9tIHRoZSBrZXJuZWwgdG9vIGxvbmcuICBJ IG1pc3MgdGhhdCBkZWxpY2F0ZQ0KPiA+ID4gPiBtYW5hZ2VtZW50IHRvdWNoLg0KPiA+ID4gDQo+ ID4gPiAiRGVsaWNhdGUgTWFuYWdlbWVudCBUb3VjaCIgaXMgbXkgbWlkZGxlIG5hbWUuDQo+ID4g PiANCj4gPiA+ID4gcGljcyBvZiB0aGUgc3RhY2sgdHJhY2UgYXQgaHR0cDovL3d3dy5tY3ZveS5j b20vbG0vbmZzLWxvY2stY3Jhc2gNCj4gPiA+IA0KPiA+ID4gT2ssIHRoYXQncyBqdXN0IHRoZSBu b3JtYWwga2luZCBvZiByYW5kb20gbGVmdC1vdmVyIG9vcHNlcyBkdWUgdG8NCj4gPiA+IHN1YnNl cXVlbnQgcHJvYmxlbXMgb2YgYSBCVUdfT04oKS4gTG9va3MgbGlrZSB0aGUgd2F0Y2hkb2cgdGlt ZXIgZW5kcw0KPiA+ID4gdXAgYmVpbmcgdW5oYXBweSwgYWxtb3N0IGNlcnRhaW5seSBzaW1wbHkg YmVjYXVzZSBzb21lIGNvcmUgZmlsZXN5c3RlbQ0KPiA+ID4gc3BpbmxvY2sgbm90IGJlaW5nIHJl bGVhc2VkLg0KPiA+ID4gDQo+ID4gPiBJdCB1c2VkIHRvIGJlIChhIGxvbmcgbG9uZyB0aW1lIGFn bykgdGhhdCB3ZSdkIHJlY292ZXIgZmFpcmx5DQo+ID4gPiBncmFjZWZ1bGx5IGZyb20gQlVHX09O KCkncyAtIGJhY2sgd2hlbiB0aGUgbWFpbiBzaGFyZWQgbG9jayB3ZSBoYWQgd2FzDQo+ID4gPiB0 aGUga2VybmVsIGxvY2ssIGFuZCB3ZSBoYWQgYSBzaW5nbGUgcGVyLXByb2Nlc3Mga2VybmVsIGxv Y2sgY291bnRlci4NCj4gPiA+IFNvIHdoZW4gd2Uga2lsbGVkIHRoZSBwcm9jZXNzLCB3ZSBjb3Vs ZCBjbGVhbiB0aGF0IHNpbmdsZSBsb2NrIHVwLg0KPiA+ID4gDQo+ID4gPiBUaGVzZSBkYXlzLCBp ZiBzb21lIHByb2Nlc3MgZGllcyBpbiByYW5kb20ga2VybmVsIGNvZGUgZHVlIHRvIGENCj4gPiA+ IEJVR19PTigpIG9yIGEgd2lsZCBwb2ludGVyIG9yIHNpbWlsYXIsIGFuZCB3ZSBraWxsIGl0LCB3 ZSBhcmUgc2VsZG9tDQo+ID4gPiBhYmxlIHRvIGRvIHNvIGNsZWFubHkuIFNvIHRoZSBiZXN0IHdl IGNhbiBob3BlIGZvciBpcyB0aGF0IGl0IGhhcHBlbmVkDQo+ID4gPiBpbiBzb21lIGNvbnRleHQg d2hlcmUgaXQgaGVsZCBubyAoaW1wb3J0YW50KSBsb2Nrcy4gV2hpY2ggaXMgcmFyZS4gU28NCj4g PiA+IEJVR19PTigpJ3MgYXJlIG9mdGVuIGZhdGFsLCBhbmQgdGhlcmUgYXJlIHRoZXNlIGtpbmRz IG9mIGRvd25zdHJlYW0NCj4gPiA+IHByb2JsZW1zIHdoZXJlIHRoZXkgZ2V0IGZsdXNoZWQgb2Zm IHRoZSBzY3JlZW4gYnkgc3Vic2VxdWVudCBpc3N1ZXMuLi4NCj4gPiANCj4gPiBJZiB0aGF0IGNv ZGUgaXMgYmVpbmcgY2FsbGVkIHVuZGVyIGEgbG9jaywgdGhlbiB3ZSBoYXZlIG90aGVyIHByb2Js ZW1zLg0KPiA+IEl0IGlzIHN0YW5kYXJkIFhEUiBjb2RlOiBpdCBzaG91bGQgYWx3YXlzIGJlIGNh bGxlZCBmcm9tIGFuIG9yZGluYXJ5DQo+ID4gcHJvY2VzcyBjb250ZXh0IHdpdGggbm8gc3BlY2lh bCBsb2NrcyBiZWluZyBoZWxkIGJ5IHRoZSBjYWxsZXJzLg0KPiA+IA0KPiA+ID4gSG8gaHVtbS4g R29vZ2xlIGRvZXNuJ3Qgc2VlbSB0byBiZSBmaW5kaW5nIGFueSBzaW1pbGFyIGJ1Zy1yZXBvcnRz LCBzbw0KPiA+ID4gdW5sZXNzIEJydWNlIG9yIFRyb25kIGdvICJBaGgsIEkga25vdyB3aGF0IGl0 J3MgYWJvdXQiLCBJIGRvIHRoaW5rIHdlDQo+ID4gPiB3b3VsZCB3YW50IHRvIGdldCBhcyBtdWNo IG1vcmUgaW5mbyBhcyBwb3NzaWJsZS4NCj4gPiANCj4gPiBOZXZlciBzZWVuIGl0IGJlZm9yZSwg YW5kIEkgc2VlIG5vIHJlYXNvbiB3aHkgaXQgc2hvdWxkIGRyYWcgdGhlIGVudGlyZQ0KPiA+IGJv eCBkb3duIHdpdGggaXQuIEl0IGlzIHBhcnQgb2YgdGhlIE5MTSBzZXJ2ZXIncyBjYWxsYmFjayBj b2RlLCBzbyB0aGVyZQ0KPiA+IGlzIG5vIGNoYW5jZSBvZiBpdCBiZWluZyBjYWxsZWQgYXMgcGFy dCBvZiBhIG1lbW9yeSByZWNsYWltIG9yIGFueXRoaW5nDQo+ID4gc2ltaWxhcmx5IHNlbnNpdGl2 ZSB0byB0aGUgcmVzdCBvZiB0aGUgYm94Lg0KPiA+IA0KPiA+IEFyZSB3ZSBzdXJlIHRoYXQgdGhp cyBCVUdfT04oKSByZWFsbHkgaXMgdG9wIG9mIHRoZSBjaGFpbiBvZiBPb3BzZXMNCj4gPiBoZXJl PyBBbGwgSSBjYW4gc2VlIGl0IGRvaW5nIGlzIGNyYXNoaW5nIHRoZSBsb2NrZCBzZXJ2ZXIgcHJv Y2Vzcywgd2hpY2gNCj4gPiB3aWxsIHNlcmlvdXNseSBpbmNvbnZlbmllbmNlIGFsbCB0aGUgTkZT IGNsaWVudHMgdHJ5aW5nIHRvIGRvIGxvY2tpbmcsDQo+ID4gYnV0IGl0IHNob3VsZG4ndCBiZSBh ZmZlY3RpbmcgdGhlIHN3YXBwZXIgcHJvY2VzcyBhcyB3ZSdyZSBzZWVpbmcgaW4gdGhlDQo+ID4g T29wcyBzY3JlZW5zaG90cy4NCj4gPiBJZiBpdCByZWFsbHkgaXMgdGhlIGZpcnN0IHRoaW5nIHRv IE9vcHMsIHRoZW4gdGhlIG9ubHkgdGhpbmcgSSBjYW4gdGhpbmsNCj4gPiBvZiB0aGVyZSB0aGF0 IHdvdWxkIHRyaWdnZXIgb3RoZXIgT29wc2VzIHdvdWxkIGJlIGEgbWVtb3J5IGNvcnJ1cHRpb24N Cj4gPiAodXNlIGFmdGVyIGZyZWUgb3Igc29tZSBzdWNoIHRoaW5nPykuIFBlcmhhcHMgTGFycnkg Y291bGQgdHJ5IHR1cm5pbmcgb24NCj4gPiBzb21lIG9mIHRoZSBsZXNzIGludHJ1c2l2ZSBzbGFi IGRlYnVnZ2luZyBvcHRpb25zPw0KPiA+IA0KPiA+ID4gRG9pbmcgYSBrZXJuZWwgY29tcGlsZSBy ZWFsbHkgaXNuJ3QgdGhhdCBiYWQuIFRoZSBvbmx5IG5hc3R5IHBpZWNlIGlzDQo+ID4gPiBnZXR0 aW5nIHRoZSBrZXJuZWwgY29uZmlndXJhdGlvbiByaWdodCwgYnV0IHlvdSBjYW4ganVzdCB1c2Ug dGhlDQo+ID4gPiBkaXN0cm8gY29uZmlnLiBJdCdzIG11Y2ggdG9vIGJpZyBhbmQgY29udGFpbnMg ZXZlcnl0aGluZywgYnV0IGl0IHdpbGwNCj4gPiA+IHdvcmssIGFuZCBnZXRzIHlvdSBhcyBzaW1p bGFyIGEga2VybmVsIGFzIHBvc3NpYmxlLiBPZiBjb3Vyc2UsIFVidW50dQ0KPiA+ID4gaGFzIG1h ZGUgaW5zdGFsbGluZyB5b3VyIG93biBrZXJuZWwgc3R1cGlkbHkgY29tcGxpY2F0ZWQgKHlvdSBo YXZlIHRvDQo+ID4gPiBidWlsZCBhIHBhY2thZ2UgYW5kIGluc3RhbGwgaXQgdXNpbmcgdGhlIHBh Y2thZ2UgbWFuYWdlciksIGJ1dCB3aGlsZQ0KPiA+ID4gaXQncyBhbiBhbm5veWluZyBleHRyYSBz dGVwIG9yIHR3byAoY29tcGFyZWQgdG8ganVzdCBkb2luZyBhICJtYWtlDQo+ID4gPiBtb2R1bGVz X2luc3RhbGwgaW5zdGFsbCIpLCBpdCdzIG5vdCByb2NrZXQgc3VyZ2VyeS4gVGhlcmUncyBhIGZl dyBoZWxwDQo+ID4gPiBwYWdlcyBmb3IgaXQ6DQo+ID4gPiANCj4gPiA+ICAgICBodHRwczovL2hl bHAudWJ1bnR1LmNvbS9jb21tdW5pdHkvS2VybmVsL0NvbXBpbGUNCj4gPiA+IA0KPiA+ID4gYmVp bmcgdGhlIGZpcnN0IG9uZS4NCj4gPiA+IA0KPiA+ID4gICAgICAgICAgICAgICAgIExpbnVzDQo+ ID4gDQo+ID4gLS0gDQo+ID4gVHJvbmQgTXlrbGVidXN0DQo+ID4gTGludXggTkZTIGNsaWVudCBt YWludGFpbmVyDQo+ID4gDQo+ID4gTmV0QXBwDQo+ID4gVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5j b20NCj4gPiB3d3cubmV0YXBwLmNvbQ0KPiANCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4 IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAu Y29tDQp3d3cubmV0YXBwLmNvbQ0K

Oh yeah, it happens every time. Run skype on a mac w/ your home dir NFS mounted. Skype tries to do some lock operation and the machine panics.

We've done it 4 times in a row to be sure we weren't smoking crack. No crack. Just panics.

---
Larry McVoy                lm at bitmover.com           http://www.bitkeeper.com

T24gRnJpLCAyMDEyLTEwLTEyIGF0IDE5OjU2IC0wNzAwLCBMYXJyeSBNY1ZveSB3cm90ZToNCj4g T24gU2F0LCBPY3QgMTMsIDIwMTIgYXQgMDI6NTI6NTFBTSArMDAwMCwgTXlrbGVidXN0LCBUcm9u ZCB3cm90ZToNCj4gPiBPbiBGcmksIDIwMTItMTAtMTIgYXQgMTk6MzEgLTA3MDAsIExhcnJ5IE1j Vm95IHdyb3RlOg0KPiA+ID4gQXMgSSd2ZSBzYWlkLCBkZWJ1Z2dpbmcgdGhpcyBpcyBnb2luZyB0 byBiZSBoYXJkLCBpdCBtZWFucyBzdG9wcGluZyBteQ0KPiA+ID4gY29tcGFueS4gIFdlIGNhbiBk byBvbmUgbW9yZSBjcmFzaCwgeW91IGd1eXMgY2FuIHRlbGwgbWUgd2hhdCB0byBkbywNCj4gPiA+ IGJ1dCB0aGF0J3MgYWJvdXQgaXQuDQo+ID4gDQo+ID4gRG8geW91IGhhdmUgYSByZXByb2R1Y2Vy PyANCj4gDQo+IE9oIHllYWgsIGl0IGhhcHBlbnMgZXZlcnkgdGltZS4gIFJ1biBza3lwZSBvbiBh IG1hYyB3LyB5b3VyIGhvbWUgZGlyIE5GUw0KPiBtb3VudGVkLiAgU2t5cGUgdHJpZXMgdG8gZG8g c29tZSBsb2NrIG9wZXJhdGlvbiBhbmQgdGhlIG1hY2hpbmUgcGFuaWNzLg0KPiANCj4gV2UndmUg ZG9uZSBpdCA0IHRpbWVzIGluIGEgcm93IHRvIGJlIHN1cmUgd2Ugd2VyZW4ndCBzbW9raW5nIGNy YWNrLiAgTm8NCj4gY3JhY2suICBKdXN0IHBhbmljcy4NCg0KVGhhbmtzISBUaGF0IHNob3VsZCBi ZSBlYXN5IHRvIGR1cGxpY2F0ZS4NCg0KSSdsbCBnaXZlIGl0IGEgc2hvdCB0b21vcnJvdywgYW5k IHNlZSBpZiBJIGNhbiBnZXQgYSB3aXJlc2hhcmsgdHJhY2UuLi4NCg0KLS0gDQpUcm9uZCBNeWts ZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xl YnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0K

OK. I think I can see what is happening. We need to filter the return values from nlm_lookup_file() when dealing with NLMv1 and NLMv3.

Can you please try the attached patch?

Cheers
Trond

PS: you are presumably running NFSv2 on your Macs. Odd that they should default to that...

Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com

From 1f0d4d1bb77fa0216dd2fb2041cc9b44db4697e0 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <[email protected]>
Date: Sat, 13 Oct 2012 00:30:28 -0400
Subject: [PATCH] NLM: nlm_lookup_file() may return NLMv4-specific error codes

If the filehandle is stale, or open access is denied for some reason, nlm_fopen() may return one of the NLMv4-specific error codes nlm4_stale_fh or nlm4_failed. These get passed right through nlm_lookup_file(), and so when nlmsvc_retrieve_args() calls the latter, it needs to filter the result through the cast_status() machinery.

Failure to do so, will trigger the BUG_ON() in encode_nlm_stat...

---
 fs/lockd/clntxdr.c | 2 +-
 fs/lockd/svcproc.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

index 180ac34..cf04cda 100644
@@ -223,7 +223,7 @@ static void encode_nlm_stat(struct xdr_stream *xdr,
 {
 	__be32 *p;
 
-	BUG_ON(be32_to_cpu(stat) > NLM_LCK_DENIED_GRACE_PERIOD);
+	WARN_ON_ONCE(be32_to_cpu(stat) > NLM_LCK_DENIED_GRACE_PERIOD);
 	p = xdr_reserve_space(xdr, 4);
 	*p = stat;
 }

index d27aab1..d413af3 100644
@@ -67,7 +67,8 @@ nlmsvc_retrieve_args(struct svc_rqst *rqstp, struct nlm_args *argp,
 
 	/* Obtain file pointer. Not used by FREE_ALL call. */
 	if (filp != NULL) {
-		if ((error = nlm_lookup_file(rqstp, &file, &lock->fh)) != 0)
+		error = cast_status(nlm_lookup_file(rqstp, &file, &lock->fh));
+		if (error != 0)
 			goto no_locks;
 		*filp = file;
 

1.7.11.7

fwiw, this was fixed in debian as of debianutils package 3.4.3. So it should be functional in testing (wheezy) but, alas, not in the current stable release (squeeze).

For ubuntu, i think the fixed packages should be available from "nattie narwhal" forward, but i haven't tested them directly.

For reference, the bug report in debian is at:

http://bugs.debian.org/607411

Regards,

	--dkg

I dunno if they default to that or we force that. We have had lots of problems with Linux NFS, we export our home directories but they are more or less read only.

Maybe things have gotten better but back in the day I could crash the kernel with a bk clone to a NFS directory.

We've also had problems with NFS versions, 2 has been stable for us so we tend to use that when we can.

If you want me to get a rant about how retarded the various NFS versions are I can go there. The Sun NFS people absolutely positively did not understand what RPC version numbers are for. And they invented them.

Sigh.

---
Larry McVoy                lm at bitmover.com           http://www.bitkeeper.com

Can't it be called from the rpciod workqueue? I'm not sure what happens when we hit a BUG there.

It looks like a bunch of BUG_ON's got added with an xdr rewrite in 2b061f9ef216b6d229b06267f188167fd6ab3d9b. Maybe Chuck or someone should do a 'git grep BUG fs/lockd' and figure out what those should be instead?

And I need to do the same for nfsd; I've been sloppy about using them as asserts.

--b.

Everybody falls into that trap at least once. Any objections to killing those macros and expanding that stuff out?

--b.

cause tons of *very* non-obvious downstream errors, so yeah, if it's called that way, I could easily see watchdogs firing etc.

Even just replacing them with a WARN_ON_ONCE() would probably help things. Because BUG_ON() really *really* should be a "last possible case" thing.

Linus

In my own defense, I also removed a lot of BUG_ON's in that series.

This particular one was added because "getting the endianness of the reply status code wrong" turns out to be a fairly easy problem to introduce without noticing it when making changes to NFSD or lockd server. There is a set of status codes that are already XDR-encoded, and a set that are not. A person can easily choose the wrong one to use.

I expected that such problems would be caught quickly during development if we actually checked for it in the XDR layer and barked if it is incorrect. I assumed therefore that these assertions would not be encountered by the time code gets in front of users.

I think range-check assertions in the XDR code are valuable. Whether they are done via BUG_ON or WARN_ON_ONCE is a matter of priority: BUG_ON forces you to notice the problem and address it, while WARN_ON allows the system to continue operating with the bug, but the bug can be ignored (or the WARN_ON simply removed because it is annoying).

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Bullshit.

Bullshit again.

I don't think you've ever even *seen* a BUG_ON() fire in critical code, have you? It's not pretty.

It doesn't "force you to notice the problem". Rather the reverse. It often causes a totally silent death of the machine, and/or scrolled-off messages that you never see, exactly because the thing you *do* see is the fallout from the developer having done something incredibly stupid.

Really.

There's a reason I say that BUG_ON() is only for those cases where the machine is already totally unrecoverable. It *will* screw the machine up, usually in ways that makes it totally undebuggable.

So please stop making excuses for it. It's bad. It isn't some kind of "this forces you to notice". Really. The fact that you think it is just shows that you probably never saw them actually trigger, or you only saw them trigger in code that happened to not hold any locks etc.

So shove those BUG_ON()'s where the sun don't shine. They make things *worse*, not better. They are valid for some very core code where a major internal data structure is totally scrogged, but they are *never* valid for some kind of random assert(). Exactly because BUG_ON() is such a big hammer that using it is more likely to *destroy* the data you want to assert and show the user, rather than actually showing the user anything at all.

Linus

I've been working on the Linux kernel NFS client for 12 years. I've seen plenty of BUG_ONs fire at inopportune moments, and plenty fire right when it is useful.

The client-side XDR encoder functions don't return a return value. Upper layers can not find out there was a problem if marshalling fails. That's why we use BUG_ON here: the code really has to stop executing, especially if pointers are involved, to avoid crapping on other parts of memory or posting data to a server that could corrupt file data.

Remember, in a file system, you need to be concerned about kernel data structures _and_ what is being written to disk.

Even if we change BUG_ON to WARN_ON everywhere, there are still plenty of ways to oops in that code, and we have the same result: the machine may lock up or not write the back trace to the system log.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

The above is STILL pure bull.

Use a WARN_ON_ONCE() so that people can actually *report* the bugs sanely (see this very thread about why BUG_ON causes problems for reporters).

If you are so damned certain that you mustn't continue, make the damn function return the error code, and stop marshalling. If you don't do error handling, that's no excuse for BUG_ON.

And if you think the code gets too complicated from that, then that *still* isn't a reason to do BUG_ON(). See above.

What part of this thread do you have problems acknowledging? The undeniable *facts* (that you seem to be in total denial about) from this very thread are:

- the BUG_ON() caused an otherwise *benign* bug to result in a unusable system.

Your "to avoid crapping on other parts of memory or posting data"
argument is pretty much f*cked up, when you instead cause a fatal error!

- the BUG_ON() caused the debug data to be almost totally useless.

Look at the screen shots. Look at the lack of debug info. Just look at it.

Why are you denying reality, and bringing up arguments that are purely theoretical and shown to be wrong in reality?

BUG_ON() in a filesystem or random service is pure and utter garbage. There is no way it is ever the right thing to do. Get rid of them, and stop posting excuses for them.

Christ, we had THIS VERY SAME issue not more than a month or two ago, when the locking code had a BUG() in it that turned what should have been a simple error return into a DoS attack by normal users. See commit 8d657eb3b438.

The whole "it's better to BUG_ON() than do something unexpected" is a disease. It may be well-intentioned, but the road to hell is paved with good intentions, and saying "I don't want to do odd things" is stupid to do, when the BUG_ON() itself just causes *different* catastrophic odd things to happen.

Really: killing the machine IS NOT ANY BETTER than sending out odd packets.

Linus

Bug reports welcomed if any of those bugs are still around.

--b.

T24gU3VuLCAyMDEyLTEwLTE0IGF0IDIwOjQzIC0wNDAwLCBCcnVjZSBGaWVsZHMgd3JvdGU6DQo+ IE9uIFNhdCwgT2N0IDEzLCAyMDEyIGF0IDA2OjQyOjE1UE0gLTA3MDAsIExhcnJ5IE1jVm95IHdy b3RlOg0KPiA+ID4gUFM6IHlvdSBhcmUgcHJlc3VtYWJseSBydW5uaW5nIE5GU3YyIG9uIHlvdXIg TWFjcy4gT2RkIHRoYXQgdGhleSBzaG91bGQNCj4gPiA+IGRlZmF1bHQgdG8gdGhhdC4uLg0KPiA+ IA0KPiA+IEkgZHVubm8gaWYgdGhleSBkZWZhdWx0IHRvIHRoYXQgb3Igd2UgZm9yY2UgdGhhdC4g IFdlIGhhdmUgaGFkIGxvdHMgb2YNCj4gPiBwcm9ibGVtcyB3aXRoIExpbnV4IE5GUywgd2UgZXhw b3J0IG91ciBob21lIGRpcmVjdG9yaWVzIGJ1dCB0aGV5IGFyZQ0KPiA+IG1vcmUgb3IgbGVzcyBy ZWFkIG9ubHkuDQo+ID4gDQo+ID4gTWF5YmUgdGhpbmdzIGhhdmUgZ290dGVuIGJldHRlciBidXQg YmFjayBpbiB0aGUgZGF5IEkgY291bGQgY3Jhc2ggdGhlDQo+ID4ga2VybmVsIHdpdGggYSBiayBj bG9uZSB0byBhIE5GUyBkaXJlY3RvcnkuDQo+IA0KPiBCdWcgcmVwb3J0cyB3ZWxjb21lZCBpZiBh bnkgb2YgdGhvc2UgYnVncyBhcmUgc3RpbGwgYXJvdW5kLg0KDQpUaGUgb3RoZXIgdGhpbmcgdG8g bm90ZSBpcyB0aGF0IGF0IHRoaXMgcG9pbnQsIE5GU3YyIGhhcyBiZWVuIGxlZ2FjeQ0KY29kZSBm b3IgbW9yZSB0aGFuIDEwIHllYXJzIG9uIExpbnV4IGFuZCBpcyBzdGFydGluZyB0byBzdWZmZXIg YmlnIHRpbWUNCmZyb20gYml0IHJvdC4gV2Ugc2hvdWxkIHByb2JhYmx5IGFpbSB0byByZW1vdmUg aXQgZW50aXJlbHkgaW4gdGhlIG5leHQNCjEtMiB5ZWFycy4gVGhlcmUgaXMgbm8gcGxhY2UgaW4g dG9kYXkncyB3b3JsZCBmb3IgYSBwcm90b2NvbCB0aGF0IGNhbg0Kb25seSBkZWFsIHdpdGggYSAy R0IgbWF4aW11bSBmaWxlIHNpemUuLi4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5G UyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29t DQp3d3cubmV0YXBwLmNvbQ0K

T24gU3VuLCAyMDEyLTEwLTE0IGF0IDIwOjQzIC0wNDAwLCBCcnVjZSBGaWVsZHMgd3JvdGU6DQo+ IEJ1ZyByZXBvcnRzIHdlbGNvbWVkIGlmIGFueSBvZiB0aG9zZSBidWdzIGFyZSBzdGlsbCBhcm91 bmQuDQoNCllvdSBkaWQgc2VlIHRoZSBwYXRjaCB0aGF0IEkgc3VibWl0dGVkLCByaWdodD8gU2lu Y2UgaXQgaXMgYSBzZXJ2ZXIgYnVnLA0KSSdtIGFzc3VtaW5nIHRoYXQgeW91IHdpbGwgd2FudCB0 byBwdXNoIGl0IHVwc3RyZWFtLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNs aWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3 dy5uZXRhcHAuY29tDQo=

Yes.

Sure, I can take it.

--b.

Well, we support ancient machines (why? damn good question) that only speak NFSv2 so it would sorta suck for us if you pulled it.

---
Larry McVoy                lm at bitmover.com           http://www.bitkeeper.com

At least on the server side, I'd prefer to be more conservative about removing NFSv2 support.

--b.

No objections from me. I did the one file just to understand and actually I think the code even looks better without them.

But then I realized they are all over, I'm not sure I'll have time to do it

Thanks
Boaz

Larry, did you get a chance to test this patch?

It seems obviously right anyway, so I'll go ahead and pass it along regardless, but it would be nice to have the confirmation.

--b.