WARNING: at linux/fs/inode.c:280 drop_nlink

WARNING: at linux/fs/inode.c:280 drop_nlink

[Ricky Ng-Adam]

A one time warning after quite a bit of usage with no other problems:

------------[ cut here ]------------
WARNING: at /home/rngadam/lophilo/linux/fs/inode.c:280 drop_nlink+0x54/0x60()
Modules linked in: ipt_REDIRECT xt_tcpudp iptable_nat nf_nat
nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack ip_tables x_tables
snd_atmel_ac97c snd_ac97_codec ac97_bus snd_pcm snd_page_alloc
snd_timer snd soundcore lophilo(O) ipv6 autofs4
[<c000dfdc>] (unwind_backtrace+0x0/0xf0) from [<c0017784>]
(warn_slowpath_common+0x4c/0x64)
[<c0017784>] (warn_slowpath_common+0x4c/0x64) from [<c00177b8>]
(warn_slowpath_null+0x1c/0x24)
[<c00177b8>] (warn_slowpath_null+0x1c/0x24) from [<c00a9084>]
(drop_nlink+0x54/0x60)
[<c00a9084>] (drop_nlink+0x54/0x60) from [<c0131268>]
(nfs_dentry_iput+0x38/0x50)
[<c0131268>] (nfs_dentry_iput+0x38/0x50) from [<c00a67b0>] (d_kill+0x9c/0xec)
[<c00a67b0>] (d_kill+0x9c/0xec) from [<c00a6cf0>] (dput+0x78/0x134)
[<c00a6cf0>] (dput+0x78/0x134) from [<c0095128>] (fput+0x194/0x238)
[<c0095128>] (fput+0x194/0x238) from [<c0092380>] (filp_close+0x68/0x80)
[<c0092380>] (filp_close+0x68/0x80) from [<c001ab88>]
(put_files_struct+0xb4/0xd8)
[<c001ab88>] (put_files_struct+0xb4/0xd8) from [<c001ad1c>]
(do_exit+0x134/0x72c)
[<c001ad1c>] (do_exit+0x134/0x72c) from [<c001b59c>] (do_group_exit+0x40/0xac)
[<c001b59c>] (do_group_exit+0x40/0xac) from [<c001b618>]
(__wake_up_parent+0x0/0x18)
---[ end trace 3bda0ec3b276e040 ]---

client:

Linux lophilo 3.4.4+ #34 Sat Oct 6 16:28:57 CST 2012 armv5tejl GNU/Linux

server:

Linux rngadam-think 3.5.0-17-generic #28-Ubuntu SMP Tue Oct 9 19:32:08
UTC 2012 i686 i686 i686 GNU/Linux

# exportfs -v
/home/rngadam/lophilo.nfs
<world>(rw,async,wdelay,no_root_squash,no_subtree_check)
/home/rngadam/lophilo
<world>(rw,wdelay,root_squash,all_squash,no_subtree_check,anonuid=0,anongid=0)

seems to have occurred while a bunch of faiilng lstat on long paths
were going on:

stat { [Error: ENOENT, lstat
'/home/lophilo/lophilo/lmc/users/rngadam/lophilojs-example.git/client/code/colorpicker/img/colorpicker/img/colorpicker/img/colorpicker/js/colorpicker/css/colorpicker']
  errno: 34,
  code: 'ENOENT',
  path: '/home/lophilo/lophilo/lmc/users/rngadam/lophilojs-example.git/client/code/colorpicker/img/colorpicker/img/colorpicker/img/colorpicker/js/colorpicker/css/colorpicker'
}

--
伍思力 | Ricky Ng-Adam | Lophilo Ltd | http://lophilo.com | (+86)186-2126-2521

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Jeff Layton]

On Sat, 27 Oct 2012 20:19:38 +0800
Ricky Ng-Adam <rngadam@lophilo.com> wrote:

> A one time warning after quite a bit of usage with no other problems:
> 
> ------------[ cut here ]------------
> WARNING: at /home/rngadam/lophilo/linux/fs/inode.c:280 drop_nlink+0x54/0x60()
> Modules linked in: ipt_REDIRECT xt_tcpudp iptable_nat nf_nat
> nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack ip_tables x_tables
> snd_atmel_ac97c snd_ac97_codec ac97_bus snd_pcm snd_page_alloc
> snd_timer snd soundcore lophilo(O) ipv6 autofs4
> [<c000dfdc>] (unwind_backtrace+0x0/0xf0) from [<c0017784>]
> (warn_slowpath_common+0x4c/0x64)
> [<c0017784>] (warn_slowpath_common+0x4c/0x64) from [<c00177b8>]
> (warn_slowpath_null+0x1c/0x24)
> [<c00177b8>] (warn_slowpath_null+0x1c/0x24) from [<c00a9084>]
> (drop_nlink+0x54/0x60)
> [<c00a9084>] (drop_nlink+0x54/0x60) from [<c0131268>]
> (nfs_dentry_iput+0x38/0x50)
> [<c0131268>] (nfs_dentry_iput+0x38/0x50) from [<c00a67b0>] (d_kill+0x9c/0xec)
> [<c00a67b0>] (d_kill+0x9c/0xec) from [<c00a6cf0>] (dput+0x78/0x134)
> [<c00a6cf0>] (dput+0x78/0x134) from [<c0095128>] (fput+0x194/0x238)
> [<c0095128>] (fput+0x194/0x238) from [<c0092380>] (filp_close+0x68/0x80)
> [<c0092380>] (filp_close+0x68/0x80) from [<c001ab88>]
> (put_files_struct+0xb4/0xd8)
> [<c001ab88>] (put_files_struct+0xb4/0xd8) from [<c001ad1c>]
> (do_exit+0x134/0x72c)
> [<c001ad1c>] (do_exit+0x134/0x72c) from [<c001b59c>] (do_group_exit+0x40/0xac)
> [<c001b59c>] (do_group_exit+0x40/0xac) from [<c001b618>]
> (__wake_up_parent+0x0/0x18)
> ---[ end trace 3bda0ec3b276e040 ]---
> 
> client:
> 
> Linux lophilo 3.4.4+ #34 Sat Oct 6 16:28:57 CST 2012 armv5tejl GNU/Linux
> 
> server:
> 
> Linux rngadam-think 3.5.0-17-generic #28-Ubuntu SMP Tue Oct 9 19:32:08
> UTC 2012 i686 i686 i686 GNU/Linux
> 
> # exportfs -v
> /home/rngadam/lophilo.nfs
> <world>(rw,async,wdelay,no_root_squash,no_subtree_check)
> /home/rngadam/lophilo
> <world>(rw,wdelay,root_squash,all_squash,no_subtree_check,anonuid=0,anongid=0)
> 
> seems to have occurred while a bunch of faiilng lstat on long paths
> were going on:
> 
> stat { [Error: ENOENT, lstat
> '/home/lophilo/lophilo/lmc/users/rngadam/lophilojs-example.git/client/code/colorpicker/img/colorpicker/img/colorpicker/img/colorpicker/js/colorpicker/css/colorpicker']
>   errno: 34,
>   code: 'ENOENT',
>   path: '/home/lophilo/lophilo/lmc/users/rngadam/lophilojs-example.git/client/code/colorpicker/img/colorpicker/img/colorpicker/img/colorpicker/js/colorpicker/css/colorpicker'
> }
> 

The warning is fairly harmless, but it does look scary. Neil Brown had
a patch to fix it, but I don't think Trond ever took it or commented on
it.

Trond, any thoughts on Neil's one-line patch here?

    https://lkml.org/lkml/2012/9/25/24

(Note that I still have my doubts as to whether CIFS or NFS ought to be
manipulating or relying on the i_nlink like this, but the patch looks
fairly harmless as an interim fix).

-- 
Jeff Layton <jlayton@redhat.com>

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Jeff Layton]

On Mon, 12 Nov 2012 09:52:55 -0500
Jeff Layton <jlayton@redhat.com> wrote:

> On Sat, 27 Oct 2012 20:19:38 +0800
> Ricky Ng-Adam <rngadam@lophilo.com> wrote:
> 
> > A one time warning after quite a bit of usage with no other problems:
> > 
> > ------------[ cut here ]------------
> > WARNING: at /home/rngadam/lophilo/linux/fs/inode.c:280 drop_nlink+0x54/0x60()
> > Modules linked in: ipt_REDIRECT xt_tcpudp iptable_nat nf_nat
> > nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack ip_tables x_tables
> > snd_atmel_ac97c snd_ac97_codec ac97_bus snd_pcm snd_page_alloc
> > snd_timer snd soundcore lophilo(O) ipv6 autofs4
> > [<c000dfdc>] (unwind_backtrace+0x0/0xf0) from [<c0017784>]
> > (warn_slowpath_common+0x4c/0x64)
> > [<c0017784>] (warn_slowpath_common+0x4c/0x64) from [<c00177b8>]
> > (warn_slowpath_null+0x1c/0x24)
> > [<c00177b8>] (warn_slowpath_null+0x1c/0x24) from [<c00a9084>]
> > (drop_nlink+0x54/0x60)
> > [<c00a9084>] (drop_nlink+0x54/0x60) from [<c0131268>]
> > (nfs_dentry_iput+0x38/0x50)
> > [<c0131268>] (nfs_dentry_iput+0x38/0x50) from [<c00a67b0>] (d_kill+0x9c/0xec)
> > [<c00a67b0>] (d_kill+0x9c/0xec) from [<c00a6cf0>] (dput+0x78/0x134)
> > [<c00a6cf0>] (dput+0x78/0x134) from [<c0095128>] (fput+0x194/0x238)
> > [<c0095128>] (fput+0x194/0x238) from [<c0092380>] (filp_close+0x68/0x80)
> > [<c0092380>] (filp_close+0x68/0x80) from [<c001ab88>]
> > (put_files_struct+0xb4/0xd8)
> > [<c001ab88>] (put_files_struct+0xb4/0xd8) from [<c001ad1c>]
> > (do_exit+0x134/0x72c)
> > [<c001ad1c>] (do_exit+0x134/0x72c) from [<c001b59c>] (do_group_exit+0x40/0xac)
> > [<c001b59c>] (do_group_exit+0x40/0xac) from [<c001b618>]
> > (__wake_up_parent+0x0/0x18)
> > ---[ end trace 3bda0ec3b276e040 ]---
> > 
> > client:
> > 
> > Linux lophilo 3.4.4+ #34 Sat Oct 6 16:28:57 CST 2012 armv5tejl GNU/Linux
> > 
> > server:
> > 
> > Linux rngadam-think 3.5.0-17-generic #28-Ubuntu SMP Tue Oct 9 19:32:08
> > UTC 2012 i686 i686 i686 GNU/Linux
> > 
> > # exportfs -v
> > /home/rngadam/lophilo.nfs
> > <world>(rw,async,wdelay,no_root_squash,no_subtree_check)
> > /home/rngadam/lophilo
> > <world>(rw,wdelay,root_squash,all_squash,no_subtree_check,anonuid=0,anongid=0)
> > 
> > seems to have occurred while a bunch of faiilng lstat on long paths
> > were going on:
> > 
> > stat { [Error: ENOENT, lstat
> > '/home/lophilo/lophilo/lmc/users/rngadam/lophilojs-example.git/client/code/colorpicker/img/colorpicker/img/colorpicker/img/colorpicker/js/colorpicker/css/colorpicker']
> >   errno: 34,
> >   code: 'ENOENT',
> >   path: '/home/lophilo/lophilo/lmc/users/rngadam/lophilojs-example.git/client/code/colorpicker/img/colorpicker/img/colorpicker/img/colorpicker/js/colorpicker/css/colorpicker'
> > }
> > 
> 
> The warning is fairly harmless, but it does look scary. Neil Brown had
> a patch to fix it, but I don't think Trond ever took it or commented on
> it.
> 
> Trond, any thoughts on Neil's one-line patch here?
> 
>     https://lkml.org/lkml/2012/9/25/24
> 
> (Note that I still have my doubts as to whether CIFS or NFS ought to be
> manipulating or relying on the i_nlink like this, but the patch looks
> fairly harmless as an interim fix).
> 

Hi Trond,

I asked about this a while back and never got a response. I have a
few bugs open against Fedora on this and wouldn't mind laying this to
rest. Is there any reason not to use the nfs_drop_nlink() helper in
nfs_dentry_iput()?

-- 
Jeff Layton <jlayton@redhat.com>

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTEyLTEzIGF0IDA2OjMxIC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g
T24gTW9uLCAxMiBOb3YgMjAxMiAwOTo1Mjo1NSAtMDUwMA0KPiBKZWZmIExheXRvbiA8amxheXRv
bkByZWRoYXQuY29tPiB3cm90ZToNCj4gDQo+ID4gT24gU2F0LCAyNyBPY3QgMjAxMiAyMDoxOToz
OCArMDgwMA0KPiA+IFJpY2t5IE5nLUFkYW0gPHJuZ2FkYW1AbG9waGlsby5jb20+IHdyb3RlOg0K
PiA+IA0KPiA+ID4gQSBvbmUgdGltZSB3YXJuaW5nIGFmdGVyIHF1aXRlIGEgYml0IG9mIHVzYWdl
IHdpdGggbm8gb3RoZXIgcHJvYmxlbXM6DQo+ID4gPiANCj4gPiA+IC0tLS0tLS0tLS0tLVsgY3V0
IGhlcmUgXS0tLS0tLS0tLS0tLQ0KPiA+ID4gV0FSTklORzogYXQgL2hvbWUvcm5nYWRhbS9sb3Bo
aWxvL2xpbnV4L2ZzL2lub2RlLmM6MjgwIGRyb3BfbmxpbmsrMHg1NC8weDYwKCkNCj4gPiA+IE1v
ZHVsZXMgbGlua2VkIGluOiBpcHRfUkVESVJFQ1QgeHRfdGNwdWRwIGlwdGFibGVfbmF0IG5mX25h
dA0KPiA+ID4gbmZfY29ubnRyYWNrX2lwdjQgbmZfZGVmcmFnX2lwdjQgbmZfY29ubnRyYWNrIGlw
X3RhYmxlcyB4X3RhYmxlcw0KPiA+ID4gc25kX2F0bWVsX2FjOTdjIHNuZF9hYzk3X2NvZGVjIGFj
OTdfYnVzIHNuZF9wY20gc25kX3BhZ2VfYWxsb2MNCj4gPiA+IHNuZF90aW1lciBzbmQgc291bmRj
b3JlIGxvcGhpbG8oTykgaXB2NiBhdXRvZnM0DQo+ID4gPiBbPGMwMDBkZmRjPl0gKHVud2luZF9i
YWNrdHJhY2UrMHgwLzB4ZjApIGZyb20gWzxjMDAxNzc4ND5dDQo+ID4gPiAod2Fybl9zbG93cGF0
aF9jb21tb24rMHg0Yy8weDY0KQ0KPiA+ID4gWzxjMDAxNzc4ND5dICh3YXJuX3Nsb3dwYXRoX2Nv
bW1vbisweDRjLzB4NjQpIGZyb20gWzxjMDAxNzdiOD5dDQo+ID4gPiAod2Fybl9zbG93cGF0aF9u
dWxsKzB4MWMvMHgyNCkNCj4gPiA+IFs8YzAwMTc3Yjg+XSAod2Fybl9zbG93cGF0aF9udWxsKzB4
MWMvMHgyNCkgZnJvbSBbPGMwMGE5MDg0Pl0NCj4gPiA+IChkcm9wX25saW5rKzB4NTQvMHg2MCkN
Cj4gPiA+IFs8YzAwYTkwODQ+XSAoZHJvcF9ubGluaysweDU0LzB4NjApIGZyb20gWzxjMDEzMTI2
OD5dDQo+ID4gPiAobmZzX2RlbnRyeV9pcHV0KzB4MzgvMHg1MCkNCj4gPiA+IFs8YzAxMzEyNjg+
XSAobmZzX2RlbnRyeV9pcHV0KzB4MzgvMHg1MCkgZnJvbSBbPGMwMGE2N2IwPl0gKGRfa2lsbCsw
eDljLzB4ZWMpDQo+ID4gPiBbPGMwMGE2N2IwPl0gKGRfa2lsbCsweDljLzB4ZWMpIGZyb20gWzxj
MDBhNmNmMD5dIChkcHV0KzB4NzgvMHgxMzQpDQo+ID4gPiBbPGMwMGE2Y2YwPl0gKGRwdXQrMHg3
OC8weDEzNCkgZnJvbSBbPGMwMDk1MTI4Pl0gKGZwdXQrMHgxOTQvMHgyMzgpDQo+ID4gPiBbPGMw
MDk1MTI4Pl0gKGZwdXQrMHgxOTQvMHgyMzgpIGZyb20gWzxjMDA5MjM4MD5dIChmaWxwX2Nsb3Nl
KzB4NjgvMHg4MCkNCj4gPiA+IFs8YzAwOTIzODA+XSAoZmlscF9jbG9zZSsweDY4LzB4ODApIGZy
b20gWzxjMDAxYWI4OD5dDQo+ID4gPiAocHV0X2ZpbGVzX3N0cnVjdCsweGI0LzB4ZDgpDQo+ID4g
PiBbPGMwMDFhYjg4Pl0gKHB1dF9maWxlc19zdHJ1Y3QrMHhiNC8weGQ4KSBmcm9tIFs8YzAwMWFk
MWM+XQ0KPiA+ID4gKGRvX2V4aXQrMHgxMzQvMHg3MmMpDQo+ID4gPiBbPGMwMDFhZDFjPl0gKGRv
X2V4aXQrMHgxMzQvMHg3MmMpIGZyb20gWzxjMDAxYjU5Yz5dIChkb19ncm91cF9leGl0KzB4NDAv
MHhhYykNCj4gPiA+IFs8YzAwMWI1OWM+XSAoZG9fZ3JvdXBfZXhpdCsweDQwLzB4YWMpIGZyb20g
WzxjMDAxYjYxOD5dDQo+ID4gPiAoX193YWtlX3VwX3BhcmVudCsweDAvMHgxOCkNCj4gPiA+IC0t
LVsgZW5kIHRyYWNlIDNiZGEwZWMzYjI3NmUwNDAgXS0tLQ0KPiA+ID4gDQo+ID4gPiBjbGllbnQ6
DQo+ID4gPiANCj4gPiA+IExpbnV4IGxvcGhpbG8gMy40LjQrICMzNCBTYXQgT2N0IDYgMTY6Mjg6
NTcgQ1NUIDIwMTIgYXJtdjV0ZWpsIEdOVS9MaW51eA0KPiA+ID4gDQo+ID4gPiBzZXJ2ZXI6DQo+
ID4gPiANCj4gPiA+IExpbnV4IHJuZ2FkYW0tdGhpbmsgMy41LjAtMTctZ2VuZXJpYyAjMjgtVWJ1
bnR1IFNNUCBUdWUgT2N0IDkgMTk6MzI6MDgNCj4gPiA+IFVUQyAyMDEyIGk2ODYgaTY4NiBpNjg2
IEdOVS9MaW51eA0KPiA+ID4gDQo+ID4gPiAjIGV4cG9ydGZzIC12DQo+ID4gPiAvaG9tZS9ybmdh
ZGFtL2xvcGhpbG8ubmZzDQo+ID4gPiA8d29ybGQ+KHJ3LGFzeW5jLHdkZWxheSxub19yb290X3Nx
dWFzaCxub19zdWJ0cmVlX2NoZWNrKQ0KPiA+ID4gL2hvbWUvcm5nYWRhbS9sb3BoaWxvDQo+ID4g
PiA8d29ybGQ+KHJ3LHdkZWxheSxyb290X3NxdWFzaCxhbGxfc3F1YXNoLG5vX3N1YnRyZWVfY2hl
Y2ssYW5vbnVpZD0wLGFub25naWQ9MCkNCj4gPiA+IA0KPiA+ID4gc2VlbXMgdG8gaGF2ZSBvY2N1
cnJlZCB3aGlsZSBhIGJ1bmNoIG9mIGZhaWlsbmcgbHN0YXQgb24gbG9uZyBwYXRocw0KPiA+ID4g
d2VyZSBnb2luZyBvbjoNCj4gPiA+IA0KPiA+ID4gc3RhdCB7IFtFcnJvcjogRU5PRU5ULCBsc3Rh
dA0KPiA+ID4gJy9ob21lL2xvcGhpbG8vbG9waGlsby9sbWMvdXNlcnMvcm5nYWRhbS9sb3BoaWxv
anMtZXhhbXBsZS5naXQvY2xpZW50L2NvZGUvY29sb3JwaWNrZXIvaW1nL2NvbG9ycGlja2VyL2lt
Zy9jb2xvcnBpY2tlci9pbWcvY29sb3JwaWNrZXIvanMvY29sb3JwaWNrZXIvY3NzL2NvbG9ycGlj
a2VyJ10NCj4gPiA+ICAgZXJybm86IDM0LA0KPiA+ID4gICBjb2RlOiAnRU5PRU5UJywNCj4gPiA+
ICAgcGF0aDogJy9ob21lL2xvcGhpbG8vbG9waGlsby9sbWMvdXNlcnMvcm5nYWRhbS9sb3BoaWxv
anMtZXhhbXBsZS5naXQvY2xpZW50L2NvZGUvY29sb3JwaWNrZXIvaW1nL2NvbG9ycGlja2VyL2lt
Zy9jb2xvcnBpY2tlci9pbWcvY29sb3JwaWNrZXIvanMvY29sb3JwaWNrZXIvY3NzL2NvbG9ycGlj
a2VyJw0KPiA+ID4gfQ0KPiA+ID4gDQo+ID4gDQo+ID4gVGhlIHdhcm5pbmcgaXMgZmFpcmx5IGhh
cm1sZXNzLCBidXQgaXQgZG9lcyBsb29rIHNjYXJ5LiBOZWlsIEJyb3duIGhhZA0KPiA+IGEgcGF0
Y2ggdG8gZml4IGl0LCBidXQgSSBkb24ndCB0aGluayBUcm9uZCBldmVyIHRvb2sgaXQgb3IgY29t
bWVudGVkIG9uDQo+ID4gaXQuDQo+ID4gDQo+ID4gVHJvbmQsIGFueSB0aG91Z2h0cyBvbiBOZWls
J3Mgb25lLWxpbmUgcGF0Y2ggaGVyZT8NCj4gPiANCj4gPiAgICAgaHR0cHM6Ly9sa21sLm9yZy9s
a21sLzIwMTIvOS8yNS8yNA0KPiA+IA0KPiA+IChOb3RlIHRoYXQgSSBzdGlsbCBoYXZlIG15IGRv
dWJ0cyBhcyB0byB3aGV0aGVyIENJRlMgb3IgTkZTIG91Z2h0IHRvIGJlDQo+ID4gbWFuaXB1bGF0
aW5nIG9yIHJlbHlpbmcgb24gdGhlIGlfbmxpbmsgbGlrZSB0aGlzLCBidXQgdGhlIHBhdGNoIGxv
b2tzDQo+ID4gZmFpcmx5IGhhcm1sZXNzIGFzIGFuIGludGVyaW0gZml4KS4NCj4gPiANCj4gDQo+
IEhpIFRyb25kLA0KPiANCj4gSSBhc2tlZCBhYm91dCB0aGlzIGEgd2hpbGUgYmFjayBhbmQgbmV2
ZXIgZ290IGEgcmVzcG9uc2UuIEkgaGF2ZSBhDQo+IGZldyBidWdzIG9wZW4gYWdhaW5zdCBGZWRv
cmEgb24gdGhpcyBhbmQgd291bGRuJ3QgbWluZCBsYXlpbmcgdGhpcyB0bw0KPiByZXN0LiBJcyB0
aGVyZSBhbnkgcmVhc29uIG5vdCB0byB1c2UgdGhlIG5mc19kcm9wX25saW5rKCkgaGVscGVyIGlu
DQo+IG5mc19kZW50cnlfaXB1dCgpPw0KDQpZb3UgbWVhbiBhc2lkZSBmcm9tIHRoZSBmYWN0IHRo
YXQgc2ItPnNfcmVtb3ZlX2NvdW50IHJlbWFpbnMgYSByYWN5DQpwaWVjZSBvZiBjcmFwIHRoYXQg
c2VydmVzIG5vIGdvb2QgcHVycG9zZSBmb3IgTkZTLCBhbmQgeWV0IHdpbGwgY29udGludWUNCnRv
IGdpdmUgdXMgZ3JpZWY/DQoNCkknZCBwcmVmZXIgdG8gc2VlIGEgc29sdXRpb24gdGhhdCBnZXRz
IHJpZCBvZg0KZHJvcF9ubGluaygpL2NsZWFyX25saW5rKCkvc2V0X25saW5rKCkgYWx0b2dldGhl
ci4gVGhleSBhbGwgc3Vjay4uLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNs
aWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3
dy5uZXRhcHAuY29tDQo=

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Al Viro]

On Thu, Dec 13, 2012 at 02:45:18PM +0000, Myklebust, Trond wrote:

> You mean aside from the fact that sb->s_remove_count remains a racy
> piece of crap that serves no good purpose for NFS, and yet will continue
> to give us grief?

As opposed to scanning the list of opened files?

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTEyLTEzIGF0IDIxOjA3ICswMDAwLCBBbCBWaXJvIHdyb3RlOg0KPiBPbiBU
aHUsIERlYyAxMywgMjAxMiBhdCAwMjo0NToxOFBNICswMDAwLCBNeWtsZWJ1c3QsIFRyb25kIHdy
b3RlOg0KPiANCj4gPiBZb3UgbWVhbiBhc2lkZSBmcm9tIHRoZSBmYWN0IHRoYXQgc2ItPnNfcmVt
b3ZlX2NvdW50IHJlbWFpbnMgYSByYWN5DQo+ID4gcGllY2Ugb2YgY3JhcCB0aGF0IHNlcnZlcyBu
byBnb29kIHB1cnBvc2UgZm9yIE5GUywgYW5kIHlldCB3aWxsIGNvbnRpbnVlDQo+ID4gdG8gZ2l2
ZSB1cyBncmllZj8NCj4gDQo+IEFzIG9wcG9zZWQgdG8gc2Nhbm5pbmcgdGhlIGxpc3Qgb2Ygb3Bl
bmVkIGZpbGVzPw0KDQpGb3Igd2hhdCBpbmZvcm1hdGlvbj8gc2ItPnNfcmVtb3ZlX2NvdW50IHRl
bGxzIHlvdSBub3RoaW5nIG5ldyBhYm91dCB0aGUNCnN0YXRlIG9mIHRoZSBORlMgZmlsZXN5c3Rl
bS4NCg0KVGhpcyB3aG9sZSAiY2hlY2sgZm9yIG5saW5rID09IDAiIHRoaW5nIGlzIGF0IGJlc3Qg
YSBoZXVyaXN0aWMsIHNpbmNlDQp0aGUgZmlsZSBtYXkgcGVyc2lzdCBvciBkaXNhcHBlYXIgb24g
dGhlIHNlcnZlciBpbmRlcGVuZGVudGx5IG9mDQp3aGF0ZXZlciB3ZSBtYXkgdGhpbmsgYWJvdXQg
dGhlIHZhbHVlIG9mIGlub2RlLT5pX25saW5rLiBTZXR0aW5nIHNpcmVucw0KdG8gYmxhcmUgYW5k
IGFuZ2VscyB0byBzaW5nIHdoZW4gd2UgZ2V0IGl0IHdyb25nIChhcyB3ZSBvZnRlbiBkbykgaXMN
Cmp1c3QgcG9pbnRsZXNzLi4uDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xp
ZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3
Lm5ldGFwcC5jb20NCg==

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Jeff Layton]

On Thu, 13 Dec 2012 21:22:26 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> On Thu, 2012-12-13 at 21:07 +0000, Al Viro wrote:
> > On Thu, Dec 13, 2012 at 02:45:18PM +0000, Myklebust, Trond wrote:
> > 
> > > You mean aside from the fact that sb->s_remove_count remains a racy
> > > piece of crap that serves no good purpose for NFS, and yet will continue
> > > to give us grief?
> > 
> > As opposed to scanning the list of opened files?
> 
> For what information? sb->s_remove_count tells you nothing new about the
> state of the NFS filesystem.
> 
> This whole "check for nlink == 0" thing is at best a heuristic, since
> the file may persist or disappear on the server independently of
> whatever we may think about the value of inode->i_nlink. Setting sirens
> to blare and angels to sing when we get it wrong (as we often do) is
> just pointless...
> 

So...why do drop_nlink at all in NFS (or other similar filesystems like
CIFS)? In principle, it seems like we ought to just mark the attribute
cache invalid and assume that we'll pick up the nlink change when we
next attempt to revalidate the inode.

On that next attempt, we might find it stale, but that's something we
already have to deal with.

-- 
Jeff Layton <jlayton@redhat.com>

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTEyLTEzIGF0IDIyOjA2IC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g
T24gVGh1LCAxMyBEZWMgMjAxMiAyMToyMjoyNiArMDAwMA0KPiAiTXlrbGVidXN0LCBUcm9uZCIg
PFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tPiB3cm90ZToNCj4gDQo+ID4gT24gVGh1LCAyMDEy
LTEyLTEzIGF0IDIxOjA3ICswMDAwLCBBbCBWaXJvIHdyb3RlOg0KPiA+ID4gT24gVGh1LCBEZWMg
MTMsIDIwMTIgYXQgMDI6NDU6MThQTSArMDAwMCwgTXlrbGVidXN0LCBUcm9uZCB3cm90ZToNCj4g
PiA+IA0KPiA+ID4gPiBZb3UgbWVhbiBhc2lkZSBmcm9tIHRoZSBmYWN0IHRoYXQgc2ItPnNfcmVt
b3ZlX2NvdW50IHJlbWFpbnMgYSByYWN5DQo+ID4gPiA+IHBpZWNlIG9mIGNyYXAgdGhhdCBzZXJ2
ZXMgbm8gZ29vZCBwdXJwb3NlIGZvciBORlMsIGFuZCB5ZXQgd2lsbCBjb250aW51ZQ0KPiA+ID4g
PiB0byBnaXZlIHVzIGdyaWVmPw0KPiA+ID4gDQo+ID4gPiBBcyBvcHBvc2VkIHRvIHNjYW5uaW5n
IHRoZSBsaXN0IG9mIG9wZW5lZCBmaWxlcz8NCj4gPiANCj4gPiBGb3Igd2hhdCBpbmZvcm1hdGlv
bj8gc2ItPnNfcmVtb3ZlX2NvdW50IHRlbGxzIHlvdSBub3RoaW5nIG5ldyBhYm91dCB0aGUNCj4g
PiBzdGF0ZSBvZiB0aGUgTkZTIGZpbGVzeXN0ZW0uDQo+ID4gDQo+ID4gVGhpcyB3aG9sZSAiY2hl
Y2sgZm9yIG5saW5rID09IDAiIHRoaW5nIGlzIGF0IGJlc3QgYSBoZXVyaXN0aWMsIHNpbmNlDQo+
ID4gdGhlIGZpbGUgbWF5IHBlcnNpc3Qgb3IgZGlzYXBwZWFyIG9uIHRoZSBzZXJ2ZXIgaW5kZXBl
bmRlbnRseSBvZg0KPiA+IHdoYXRldmVyIHdlIG1heSB0aGluayBhYm91dCB0aGUgdmFsdWUgb2Yg
aW5vZGUtPmlfbmxpbmsuIFNldHRpbmcgc2lyZW5zDQo+ID4gdG8gYmxhcmUgYW5kIGFuZ2VscyB0
byBzaW5nIHdoZW4gd2UgZ2V0IGl0IHdyb25nIChhcyB3ZSBvZnRlbiBkbykgaXMNCj4gPiBqdXN0
IHBvaW50bGVzcy4uLg0KPiA+IA0KPiANCj4gU28uLi53aHkgZG8gZHJvcF9ubGluayBhdCBhbGwg
aW4gTkZTIChvciBvdGhlciBzaW1pbGFyIGZpbGVzeXN0ZW1zIGxpa2UNCj4gQ0lGUyk/IEluIHBy
aW5jaXBsZSwgaXQgc2VlbXMgbGlrZSB3ZSBvdWdodCB0byBqdXN0IG1hcmsgdGhlIGF0dHJpYnV0
ZQ0KPiBjYWNoZSBpbnZhbGlkIGFuZCBhc3N1bWUgdGhhdCB3ZSdsbCBwaWNrIHVwIHRoZSBubGlu
ayBjaGFuZ2Ugd2hlbiB3ZQ0KPiBuZXh0IGF0dGVtcHQgdG8gcmV2YWxpZGF0ZSB0aGUgaW5vZGUu
DQo+IA0KPiBPbiB0aGF0IG5leHQgYXR0ZW1wdCwgd2UgbWlnaHQgZmluZCBpdCBzdGFsZSwgYnV0
IHRoYXQncyBzb21ldGhpbmcgd2UNCj4gYWxyZWFkeSBoYXZlIHRvIGRlYWwgd2l0aC4NCg0KQXQg
dGhpcyBwb2ludCwgSSBkb24ndCBjYXJlLiBNeSBhcmd1bWVudCBpcyB0aGF0IEkgaGF2ZW4ndCBz
ZWVuIGEgU0lOR0xFDQppbnN0YW5jZSB3aGVyZSB0aGlzIHdhcm5pbmcgaGFzIGxlZCB0byB0aGUg
ZGlzY292ZXJ5IG9mIGEgcG9zaXRpdmUgY2FzZS4NCkV2ZXJ5IHNpbmdsZSBzdGFjayBkdW1wIHRo
YXQgSSd2ZSBzZWVuIHNvIGZhciBoYXMgYmVlbiBhIGZhbHNlIG5lZ2F0aXZlLA0Kd2hpY2gsIGFz
IGZhciBhcyBJJ20gY29uY2VybmVkLCBzaG91bGQgYmUgY29uc2lkZXJlZCBhIHJlZ3Jlc3Npb24u
DQoNCkZvcmdpdmUgbWUgZm9yIGJlaW5nIHBpc3NlZCBvZmYuLi4NCg0KLS0gDQpUcm9uZCBNeWts
ZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xl
YnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0K

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Jeff Layton]

On Fri, 14 Dec 2012 03:22:20 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> On Thu, 2012-12-13 at 22:06 -0500, Jeff Layton wrote:
> > On Thu, 13 Dec 2012 21:22:26 +0000
> > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> > 
> > > On Thu, 2012-12-13 at 21:07 +0000, Al Viro wrote:
> > > > On Thu, Dec 13, 2012 at 02:45:18PM +0000, Myklebust, Trond wrote:
> > > > 
> > > > > You mean aside from the fact that sb->s_remove_count remains a racy
> > > > > piece of crap that serves no good purpose for NFS, and yet will continue
> > > > > to give us grief?
> > > > 
> > > > As opposed to scanning the list of opened files?
> > > 
> > > For what information? sb->s_remove_count tells you nothing new about the
> > > state of the NFS filesystem.
> > > 
> > > This whole "check for nlink == 0" thing is at best a heuristic, since
> > > the file may persist or disappear on the server independently of
> > > whatever we may think about the value of inode->i_nlink. Setting sirens
> > > to blare and angels to sing when we get it wrong (as we often do) is
> > > just pointless...
> > > 
> > 
> > So...why do drop_nlink at all in NFS (or other similar filesystems like
> > CIFS)? In principle, it seems like we ought to just mark the attribute
> > cache invalid and assume that we'll pick up the nlink change when we
> > next attempt to revalidate the inode.
> > 
> > On that next attempt, we might find it stale, but that's something we
> > already have to deal with.
> 
> At this point, I don't care. My argument is that I haven't seen a SINGLE
> instance where this warning has led to the discovery of a positive case.
> Every single stack dump that I've seen so far has been a false negative,
> which, as far as I'm concerned, should be considered a regression.
> 
> Forgive me for being pissed off...
> 

I can understand that. We are getting TONS of these reports in Fedora.
Just yesterday Nick Bowler re-reported it on the linux-nfs ml. I
wouldn't mind seeing these warnings go away, or at least have some
mechanism for filesystems to opt out of them.

OTOH, there is at least a minor problem here with letting i_nlink
underflow. When we finally get around to iput_final, generic_drop_inode
is going to return false and we're going to end up with the inode
lingering in the cache longer than it really should. Presumably memory
pressure will eventually push it out, but it would be better not to
have to wait for that.

I'll also note that we call nfs_drop_nlink to decrement i_nlink
everywhere else aside from this call site. What makes nfs_dentry_iput
special in this regard?

-- 
Jeff Layton <jlayton@redhat.com>

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Myklebust, Trond]

T24gRnJpLCAyMDEyLTEyLTE0IGF0IDA3OjUxIC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g
T1RPSCwgdGhlcmUgaXMgYXQgbGVhc3QgYSBtaW5vciBwcm9ibGVtIGhlcmUgd2l0aCBsZXR0aW5n
IGlfbmxpbmsNCj4gdW5kZXJmbG93LiBXaGVuIHdlIGZpbmFsbHkgZ2V0IGFyb3VuZCB0byBpcHV0
X2ZpbmFsLCBnZW5lcmljX2Ryb3BfaW5vZGUNCj4gaXMgZ29pbmcgdG8gcmV0dXJuIGZhbHNlIGFu
ZCB3ZSdyZSBnb2luZyB0byBlbmQgdXAgd2l0aCB0aGUgaW5vZGUNCj4gbGluZ2VyaW5nIGluIHRo
ZSBjYWNoZSBsb25nZXIgdGhhbiBpdCByZWFsbHkgc2hvdWxkLiBQcmVzdW1hYmx5IG1lbW9yeQ0K
PiBwcmVzc3VyZSB3aWxsIGV2ZW50dWFsbHkgcHVzaCBpdCBvdXQsIGJ1dCBpdCB3b3VsZCBiZSBi
ZXR0ZXIgbm90IHRvDQo+IGhhdmUgdG8gd2FpdCBmb3IgdGhhdC4NCg0KQXMgSSBzYWlkLCB0aGUg
d2hvbGUgbmxpbmsgdGVzdCB0aGluZyBpcyBhIGhldXJpc3RpYyBvbiBORlMuIEp1c3QNCmJlY2F1
c2Ugd2UgdGhpbmsgd2UndmUgc3VjY2Vzc2Z1bGx5IHNlbnQgYSBSRU1PVkUgdG8gdGhlIHNlcnZl
ciwgaXQNCmRvZXNuJ3QgbWVhbiB0aGF0IGZpbGUgaGFzIGFjdHVhbGx5IGJlZW4gZGVsZXRlZC4g
UkVNT1ZFIHJlZmVycyB0byB0aGUNCmZpbGUgYnkgbmFtZSwgc28gdGhlcmUgaXMgcGxlbnR5IG9m
IG9wcG9ydHVuaXR5IGZvciB0aGUgc2VydmVyIHRvIHBsYXkNCnRyaWNrcyBvbiB1cy4gSSdtIGFz
c3VtaW5nIHRoYXQgaXMgd2hhdCBpcyBoYXBwZW5pbmcgaW4geW91ciBGZWRvcmEgYnVnDQpyZXBv
cnRzLg0KDQpBcyBmYXIgYXMgd2UncmUgY29uY2VybmVkLCB0aGUgb25seSByZWxpYWJsZSBpbmRp
Y2F0b3IgdGhhdCBhIGZpbGUgaGFzDQpiZWVuIGRlbGV0ZWQgaXMgd2hlbiB0aGUgc2VydmVyIHN0
YXJ0cyByZXBseWluZyBFU1RBTEUgdG8gdGhhdA0KZmlsZWhhbmRsZS4NCg0KPiBJJ2xsIGFsc28g
bm90ZSB0aGF0IHdlIGNhbGwgbmZzX2Ryb3BfbmxpbmsgdG8gZGVjcmVtZW50IGlfbmxpbmsNCj4g
ZXZlcnl3aGVyZSBlbHNlIGFzaWRlIGZyb20gdGhpcyBjYWxsIHNpdGUuIFdoYXQgbWFrZXMgbmZz
X2RlbnRyeV9pcHV0DQo+IHNwZWNpYWwgaW4gdGhpcyByZWdhcmQ/DQoNCm5mc19kZW50cnlfaXB1
dCgpIGlzIG5vdCBzcGVjaWFsLCBidXQgdGhlIHRlc3QgaW4gbmZzX2Ryb3BfbmxpbmsoKSBpcy4N
CklmIHdlJ3JlIG5vdCBhYmxlIHRvIHRyYWNrIGlub2RlLT5pX25saW5rLCB0aGVuIHdoeSBpcyBm
b3JjaW5nIGFuIGlub2RlDQpldmljdGlvbiBtb3JlIGNvcnJlY3QgdGhhbiBub3QgZG9pbmcgc28/
DQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0K
TmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg==

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Jeff Layton]

On Fri, 14 Dec 2012 18:22:27 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> On Fri, 2012-12-14 at 07:51 -0500, Jeff Layton wrote:
> > OTOH, there is at least a minor problem here with letting i_nlink
> > underflow. When we finally get around to iput_final, generic_drop_inode
> > is going to return false and we're going to end up with the inode
> > lingering in the cache longer than it really should. Presumably memory
> > pressure will eventually push it out, but it would be better not to
> > have to wait for that.
> 
> As I said, the whole nlink test thing is a heuristic on NFS. Just
> because we think we've successfully sent a REMOVE to the server, it
> doesn't mean that file has actually been deleted. REMOVE refers to the
> file by name, so there is plenty of opportunity for the server to play
> tricks on us. I'm assuming that is what is happening in your Fedora bug
> reports.
> 
> As far as we're concerned, the only reliable indicator that a file has
> been deleted is when the server starts replying ESTALE to that
> filehandle.
> 
> > I'll also note that we call nfs_drop_nlink to decrement i_nlink
> > everywhere else aside from this call site. What makes nfs_dentry_iput
> > special in this regard?
> 
> nfs_dentry_iput() is not special, but the test in nfs_drop_nlink() is.
> If we're not able to track inode->i_nlink, then why is forcing an inode
> eviction more correct than not doing so?
> 

The patchset you sent after the above seems basically correct to me,
but since you asked...

It's hard to generalize on server behavior, but if a server sends us an
attributes with i_nlink == 0, it seems unlikely to go positive again.
For most servers, that means that the inode is now unreachable via
LOOKUP. Therefore, once d_iput is called we won't have a way to get to
the inode again. Forcing it out of the cache seems like the right
thing to do in that case.

A negative i_nlink OTOH makes no sense at all. If our actions are going
to make that happen then we ought to take steps to prevent it.

-- 
Jeff Layton <jlayton@redhat.com>

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Myklebust, Trond]

On Mon, 2012-12-17 at 08:08 -0500, Jeff Layton wrote:
> On Fri, 14 Dec 2012 18:22:27 +0000
> "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> 
> > On Fri, 2012-12-14 at 07:51 -0500, Jeff Layton wrote:
> > > OTOH, there is at least a minor problem here with letting i_nlink
> > > underflow. When we finally get around to iput_final, generic_drop_inode
> > > is going to return false and we're going to end up with the inode
> > > lingering in the cache longer than it really should. Presumably memory
> > > pressure will eventually push it out, but it would be better not to
> > > have to wait for that.
> > 
> > As I said, the whole nlink test thing is a heuristic on NFS. Just
> > because we think we've successfully sent a REMOVE to the server, it
> > doesn't mean that file has actually been deleted. REMOVE refers to the
> > file by name, so there is plenty of opportunity for the server to play
> > tricks on us. I'm assuming that is what is happening in your Fedora bug
> > reports.
> > 
> > As far as we're concerned, the only reliable indicator that a file has
> > been deleted is when the server starts replying ESTALE to that
> > filehandle.
> > 
> > > I'll also note that we call nfs_drop_nlink to decrement i_nlink
> > > everywhere else aside from this call site. What makes nfs_dentry_iput
> > > special in this regard?
> > 
> > nfs_dentry_iput() is not special, but the test in nfs_drop_nlink() is.
> > If we're not able to track inode->i_nlink, then why is forcing an inode
> > eviction more correct than not doing so?
> > 
> 
> The patchset you sent after the above seems basically correct to me,
> but since you asked...
> 
> It's hard to generalize on server behavior, but if a server sends us an
> attributes with i_nlink == 0, it seems unlikely to go positive again.
> For most servers, that means that the inode is now unreachable via
> LOOKUP. Therefore, once d_iput is called we won't have a way to get to
> the inode again. Forcing it out of the cache seems like the right
> thing to do in that case.

We don't know what the server's idea of inode->i_nlink is. The REMOVE
operation doesn't return any information about the target inode, so we
were just manipulating our cached values.

> A negative i_nlink OTOH makes no sense at all. If our actions are going
> to make that happen then we ought to take steps to prevent it.

We now only manipulate the cached value if we want the VFS to forget the
inode. Otherwise, we just mark the inode attributes for revalidation.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: WARNING: at linux/fs/inode.c:280 drop_nlink

[Jeff Layton]

On Mon, 17 Dec 2012 15:14:29 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> On Mon, 2012-12-17 at 08:08 -0500, Jeff Layton wrote:
> > On Fri, 14 Dec 2012 18:22:27 +0000
> > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> > 
> > > On Fri, 2012-12-14 at 07:51 -0500, Jeff Layton wrote:
> > > > OTOH, there is at least a minor problem here with letting i_nlink
> > > > underflow. When we finally get around to iput_final, generic_drop_inode
> > > > is going to return false and we're going to end up with the inode
> > > > lingering in the cache longer than it really should. Presumably memory
> > > > pressure will eventually push it out, but it would be better not to
> > > > have to wait for that.
> > > 
> > > As I said, the whole nlink test thing is a heuristic on NFS. Just
> > > because we think we've successfully sent a REMOVE to the server, it
> > > doesn't mean that file has actually been deleted. REMOVE refers to the
> > > file by name, so there is plenty of opportunity for the server to play
> > > tricks on us. I'm assuming that is what is happening in your Fedora bug
> > > reports.
> > > 
> > > As far as we're concerned, the only reliable indicator that a file has
> > > been deleted is when the server starts replying ESTALE to that
> > > filehandle.
> > > 
> > > > I'll also note that we call nfs_drop_nlink to decrement i_nlink
> > > > everywhere else aside from this call site. What makes nfs_dentry_iput
> > > > special in this regard?
> > > 
> > > nfs_dentry_iput() is not special, but the test in nfs_drop_nlink() is.
> > > If we're not able to track inode->i_nlink, then why is forcing an inode
> > > eviction more correct than not doing so?
> > > 
> > 
> > The patchset you sent after the above seems basically correct to me,
> > but since you asked...
> > 
> > It's hard to generalize on server behavior, but if a server sends us an
> > attributes with i_nlink == 0, it seems unlikely to go positive again.
> > For most servers, that means that the inode is now unreachable via
> > LOOKUP. Therefore, once d_iput is called we won't have a way to get to
> > the inode again. Forcing it out of the cache seems like the right
> > thing to do in that case.
> 
> We don't know what the server's idea of inode->i_nlink is. The REMOVE
> operation doesn't return any information about the target inode, so we
> were just manipulating our cached values.
> 

Neil's reproducer is somewhat synthetic, since it involves removing
files that have been sillyrenamed. I tend to think that most
applications don't do that however...

My assumption on this problem (maybe a wrong one) is that this usually
happens when we have an out-of-order attribute update that raced in
while we're processing the REMOVE.

IOW, we have a race where the REMOVE got processed on the server before
(e.g.) a GETATTR, but the client processed the replies in opposite
order, for whatever reason.

> > A negative i_nlink OTOH makes no sense at all. If our actions are going
> > to make that happen then we ought to take steps to prevent it.
> 
> We now only manipulate the cached value if we want the VFS to forget the
> inode. Otherwise, we just mark the inode attributes for revalidation.
> 

Right. That seems reasonable.

-- 
Jeff Layton <jlayton@redhat.com>

[PATCH 1/2] NFS: Ensure that we always drop inodes that have been marked as stale

[Trond Myklebust]

There is no need to cache stale inodes.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/inode.c     | 6 ++++++
 fs/nfs/internal.h  | 1 +
 fs/nfs/nfs4super.c | 1 +
 fs/nfs/super.c     | 1 +
 4 files changed, 9 insertions(+)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 117183b..2faae14 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -107,6 +107,12 @@ u64 nfs_compat_user_ino64(u64 fileid)
 	return ino;
 }
 
+int nfs_drop_inode(struct inode *inode)
+{
+	return NFS_STALE(inode) || generic_drop_inode(inode);
+}
+EXPORT_SYMBOL_GPL(nfs_drop_inode);
+
 void nfs_clear_inode(struct inode *inode)
 {
 	/*
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 89c1ee4..f0e6c7d 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -296,6 +296,7 @@ extern struct workqueue_struct *nfsiod_workqueue;
 extern struct inode *nfs_alloc_inode(struct super_block *sb);
 extern void nfs_destroy_inode(struct inode *);
 extern int nfs_write_inode(struct inode *, struct writeback_control *);
+extern int nfs_drop_inode(struct inode *);
 extern void nfs_clear_inode(struct inode *);
 extern void nfs_evict_inode(struct inode *);
 void nfs_zap_acl_cache(struct inode *inode);
diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
index bd61221..84d2e9e 100644
--- a/fs/nfs/nfs4super.c
+++ b/fs/nfs/nfs4super.c
@@ -51,6 +51,7 @@ static const struct super_operations nfs4_sops = {
 	.alloc_inode	= nfs_alloc_inode,
 	.destroy_inode	= nfs_destroy_inode,
 	.write_inode	= nfs4_write_inode,
+	.drop_inode	= nfs_drop_inode,
 	.put_super	= nfs_put_super,
 	.statfs		= nfs_statfs,
 	.evict_inode	= nfs4_evict_inode,
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index e12cea4..aa5315b 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -308,6 +308,7 @@ const struct super_operations nfs_sops = {
 	.alloc_inode	= nfs_alloc_inode,
 	.destroy_inode	= nfs_destroy_inode,
 	.write_inode	= nfs_write_inode,
+	.drop_inode	= nfs_drop_inode,
 	.put_super	= nfs_put_super,
 	.statfs		= nfs_statfs,
 	.evict_inode	= nfs_evict_inode,
-- 
1.7.11.7

[PATCH 2/2] NFS: Fix calls to drop_nlink()

[Trond Myklebust]

It is almost always wrong for NFS to call drop_nlink() after removing a
file. What we really want is to mark the inode's attributes for
revalidation, and we want to ensure that the VFS drops it if we're
reasonably sure that this is the final unlink().
Do the former using the usual cache validity flags, and the latter
by testing if inode->i_nlink == 1, and clearing it in that case.

This also fixes the following warning reported by Neil Brown and
Jeff Layton (among others).

[634155.004438] WARNING:
at /home/abuild/rpmbuild/BUILD/kernel-desktop-3.5.0/lin [634155.004442]
Hardware name: Latitude E6510 [634155.004577]  crc_itu_t crc32c_intel
snd_hwdep snd_pcm snd_timer snd soundcor [634155.004609] Pid: 13402, comm:
bash Tainted: G        W    3.5.0-36-desktop # [634155.004611] Call Trace:
[634155.004630]  [<ffffffff8100444a>] dump_trace+0xaa/0x2b0
[634155.004641]  [<ffffffff815a23dc>] dump_stack+0x69/0x6f
[634155.004653]  [<ffffffff81041a0b>] warn_slowpath_common+0x7b/0xc0
[634155.004662]  [<ffffffff811832e4>] drop_nlink+0x34/0x40
[634155.004687]  [<ffffffffa05bb6c3>] nfs_dentry_iput+0x33/0x70 [nfs]
[634155.004714]  [<ffffffff8118049e>] dput+0x12e/0x230
[634155.004726]  [<ffffffff8116b230>] __fput+0x170/0x230
[634155.004735]  [<ffffffff81167c0f>] filp_close+0x5f/0x90
[634155.004743]  [<ffffffff81167cd7>] sys_close+0x97/0x100
[634155.004754]  [<ffffffff815c3b39>] system_call_fastpath+0x16/0x1b
[634155.004767]  [<00007f2a73a0d110>] 0x7f2a73a0d10f

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/dir.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ce8cb92..a46a746 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1155,11 +1155,14 @@ static int nfs_dentry_delete(const struct dentry *dentry)
 
 }
 
+/* Ensure that we revalidate inode->i_nlink */
 static void nfs_drop_nlink(struct inode *inode)
 {
 	spin_lock(&inode->i_lock);
-	if (inode->i_nlink > 0)
-		drop_nlink(inode);
+	/* drop the inode if we're reasonably sure this is the last link */
+	if (inode->i_nlink == 1)
+		clear_nlink(inode);
+	NFS_I(inode)->cache_validity |= NFS_INO_INVALID_ATTR;
 	spin_unlock(&inode->i_lock);
 }
 
@@ -1174,8 +1177,8 @@ static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode)
 		NFS_I(inode)->cache_validity |= NFS_INO_INVALID_DATA;
 
 	if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
-		drop_nlink(inode);
 		nfs_complete_unlink(dentry, inode);
+		nfs_drop_nlink(inode);
 	}
 	iput(inode);
 }
@@ -1646,10 +1649,8 @@ static int nfs_safe_remove(struct dentry *dentry)
 	if (inode != NULL) {
 		NFS_PROTO(inode)->return_delegation(inode);
 		error = NFS_PROTO(dir)->remove(dir, &dentry->d_name);
-		/* The VFS may want to delete this inode */
 		if (error == 0)
 			nfs_drop_nlink(inode);
-		nfs_mark_for_revalidate(inode);
 	} else
 		error = NFS_PROTO(dir)->remove(dir, &dentry->d_name);
 	if (error == -ENOENT)
-- 
1.7.11.7

Re: [PATCH 1/2] NFS: Ensure that we always drop inodes that have been marked as stale

[Jeff Layton]

On Fri, 14 Dec 2012 16:53:00 -0500
Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> There is no need to cache stale inodes.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  fs/nfs/inode.c     | 6 ++++++
>  fs/nfs/internal.h  | 1 +
>  fs/nfs/nfs4super.c | 1 +
>  fs/nfs/super.c     | 1 +
>  4 files changed, 9 insertions(+)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 117183b..2faae14 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -107,6 +107,12 @@ u64 nfs_compat_user_ino64(u64 fileid)
>  	return ino;
>  }
>  
> +int nfs_drop_inode(struct inode *inode)
> +{
> +	return NFS_STALE(inode) || generic_drop_inode(inode);
> +}
> +EXPORT_SYMBOL_GPL(nfs_drop_inode);
> +
>  void nfs_clear_inode(struct inode *inode)
>  {
>  	/*
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 89c1ee4..f0e6c7d 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -296,6 +296,7 @@ extern struct workqueue_struct *nfsiod_workqueue;
>  extern struct inode *nfs_alloc_inode(struct super_block *sb);
>  extern void nfs_destroy_inode(struct inode *);
>  extern int nfs_write_inode(struct inode *, struct writeback_control *);
> +extern int nfs_drop_inode(struct inode *);
>  extern void nfs_clear_inode(struct inode *);
>  extern void nfs_evict_inode(struct inode *);
>  void nfs_zap_acl_cache(struct inode *inode);
> diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
> index bd61221..84d2e9e 100644
> --- a/fs/nfs/nfs4super.c
> +++ b/fs/nfs/nfs4super.c
> @@ -51,6 +51,7 @@ static const struct super_operations nfs4_sops = {
>  	.alloc_inode	= nfs_alloc_inode,
>  	.destroy_inode	= nfs_destroy_inode,
>  	.write_inode	= nfs4_write_inode,
> +	.drop_inode	= nfs_drop_inode,
>  	.put_super	= nfs_put_super,
>  	.statfs		= nfs_statfs,
>  	.evict_inode	= nfs4_evict_inode,
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index e12cea4..aa5315b 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -308,6 +308,7 @@ const struct super_operations nfs_sops = {
>  	.alloc_inode	= nfs_alloc_inode,
>  	.destroy_inode	= nfs_destroy_inode,
>  	.write_inode	= nfs_write_inode,
> +	.drop_inode	= nfs_drop_inode,
>  	.put_super	= nfs_put_super,
>  	.statfs		= nfs_statfs,
>  	.evict_inode	= nfs_evict_inode,

Testing with both of these patches shows that it does fix the
reproducer that Neil came up with.

-- 
Jeff Layton <jlayton@redhat.com>