I saw the following kernel warning:
[ 1852.321222] ------------[ cut here ]------------
[ 1852.326527] WARNING: CPU: 0 PID: 118 at fs/proc/generic.c:521 remove_proc_entry+0x154/0x16b()
[ 1852.335630] remove_proc_entry: removing non-empty directory 'fs/nfsfs', leaking at least 'volumes'
[ 1852.344084] CPU: 0 PID: 118 Comm: kworker/u8:2 Not tainted 3.16.0+ #540
[ 1852.350036] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 1852.354992] Workqueue: netns cleanup_net
[ 1852.358701] 0000000000000000 ffff880116f2fbd0 ffffffff819c03e9 ffff880116f2fc18
[ 1852.366474] ffff880116f2fc08 ffffffff810744ee ffffffff811e0e6e ffff8800d4e96238
[ 1852.373507] ffffffff81dbe665 ffff8800d46a5948 0000000000000005 ffff880116f2fc68
[ 1852.380224] Call Trace:
[ 1852.381976] [<ffffffff819c03e9>] dump_stack+0x4d/0x66
[ 1852.385495] [<ffffffff810744ee>] warn_slowpath_common+0x7a/0x93
[ 1852.389869] [<ffffffff811e0e6e>] ? remove_proc_entry+0x154/0x16b
[ 1852.393987] [<ffffffff8107457b>] warn_slowpath_fmt+0x4c/0x4e
[ 1852.397999] [<ffffffff811e0e6e>] remove_proc_entry+0x154/0x16b
[ 1852.402034] [<ffffffff8129c73d>] nfs_fs_proc_net_exit+0x53/0x56
[ 1852.406136] [<ffffffff812a103b>] nfs_net_exit+0x12/0x1d
[ 1852.409774] [<ffffffff81785bc9>] ops_exit_list+0x44/0x55
[ 1852.413529] [<ffffffff81786389>] cleanup_net+0xee/0x182
[ 1852.417198] [<ffffffff81088c9e>] process_one_work+0x209/0x40d
[ 1852.502320] [<ffffffff81088bf7>] ? process_one_work+0x162/0x40d
[ 1852.587629] [<ffffffff810890c1>] worker_thread+0x1f0/0x2c7
[ 1852.673291] [<ffffffff81088ed1>] ? process_scheduled_works+0x2f/0x2f
[ 1852.759470] [<ffffffff8108e079>] kthread+0xc9/0xd1
[ 1852.843099] [<ffffffff8109427f>] ? finish_task_switch+0x3a/0xce
[ 1852.926518] [<ffffffff8108dfb0>] ? __kthread_parkme+0x61/0x61
[ 1853.008565] [<ffffffff819cbeac>] ret_from_fork+0x7c/0xb0
[ 1853.076477] [<ffffffff8108dfb0>] ? __kthread_parkme+0x61/0x61
[ 1853.140653] ---[ end trace 69c4c6617f78e32d ]---
It looks wrong that we add "/proc/net/nfsfs" in nfs_fs_proc_net_init()
while remove "/proc/fs/nfsfs" in nfs_fs_proc_net_exit().
Fixes: commit 65b38851a17 (NFS: Fix /proc/fs/nfsfs/servers and /proc/fs/nfsfs/volumes)
Cc: Eric W. Biederman <[email protected]>
Cc: Trond Myklebust <[email protected]>
Cc: Dan Aloni <[email protected]>
Signed-off-by: Cong Wang <[email protected]>
---
v2 - fix nfs_fs_proc_net_init() as well
fs/nfs/client.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 1c5ff6d..c117b96 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1418,7 +1418,7 @@ int nfs_fs_proc_net_init(struct net *net)
error_2:
remove_proc_entry("servers", nn->proc_nfsfs);
error_1:
- remove_proc_entry("fs/nfsfs", NULL);
+ remove_proc_entry("nfsfs", net->proc_net);
error_0:
return -ENOMEM;
}
@@ -1429,7 +1429,7 @@ void nfs_fs_proc_net_exit(struct net *net)
remove_proc_entry("volumes", nn->proc_nfsfs);
remove_proc_entry("servers", nn->proc_nfsfs);
- remove_proc_entry("fs/nfsfs", NULL);
+ remove_proc_entry("nfsfs", net->proc_net);
}
/*
On Wed, Aug 20, 2014 at 12:20 AM, Eric W. Biederman
<[email protected]> wrote:
> Cong Wang <[email protected]> writes:
>
>> I saw the following kernel warning:
>
> Cong thanks for finding and tracking this. I was clearly asleep at the
> switch when I was testing my fix to the nfs client code :(
>
> I have applied this patch and will push it to Linus after it has a
> little bit to sit in linux-next.
>
This is precisely _WHY_ it is supposed to go through the maintainer,
and not through arbitrary namespace trees. I'm tired of the namespace
folks pushing crap and just assuming that a "Cc:" is OK. A "Cc:
maintainer" means this is an RFC, not something that is to be
committed.
Trond
Cong Wang <[email protected]> writes:
> I saw the following kernel warning:
Cong thanks for finding and tracking this. I was clearly asleep at the
switch when I was testing my fix to the nfs client code :(
I have applied this patch and will push it to Linus after it has a
little bit to sit in linux-next.
Eric
> [ 1852.321222] ------------[ cut here ]------------
> [ 1852.326527] WARNING: CPU: 0 PID: 118 at fs/proc/generic.c:521 remove_proc_entry+0x154/0x16b()
> [ 1852.335630] remove_proc_entry: removing non-empty directory 'fs/nfsfs', leaking at least 'volumes'
> [ 1852.344084] CPU: 0 PID: 118 Comm: kworker/u8:2 Not tainted 3.16.0+ #540
> [ 1852.350036] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 1852.354992] Workqueue: netns cleanup_net
> [ 1852.358701] 0000000000000000 ffff880116f2fbd0 ffffffff819c03e9 ffff880116f2fc18
> [ 1852.366474] ffff880116f2fc08 ffffffff810744ee ffffffff811e0e6e ffff8800d4e96238
> [ 1852.373507] ffffffff81dbe665 ffff8800d46a5948 0000000000000005 ffff880116f2fc68
> [ 1852.380224] Call Trace:
> [ 1852.381976] [<ffffffff819c03e9>] dump_stack+0x4d/0x66
> [ 1852.385495] [<ffffffff810744ee>] warn_slowpath_common+0x7a/0x93
> [ 1852.389869] [<ffffffff811e0e6e>] ? remove_proc_entry+0x154/0x16b
> [ 1852.393987] [<ffffffff8107457b>] warn_slowpath_fmt+0x4c/0x4e
> [ 1852.397999] [<ffffffff811e0e6e>] remove_proc_entry+0x154/0x16b
> [ 1852.402034] [<ffffffff8129c73d>] nfs_fs_proc_net_exit+0x53/0x56
> [ 1852.406136] [<ffffffff812a103b>] nfs_net_exit+0x12/0x1d
> [ 1852.409774] [<ffffffff81785bc9>] ops_exit_list+0x44/0x55
> [ 1852.413529] [<ffffffff81786389>] cleanup_net+0xee/0x182
> [ 1852.417198] [<ffffffff81088c9e>] process_one_work+0x209/0x40d
> [ 1852.502320] [<ffffffff81088bf7>] ? process_one_work+0x162/0x40d
> [ 1852.587629] [<ffffffff810890c1>] worker_thread+0x1f0/0x2c7
> [ 1852.673291] [<ffffffff81088ed1>] ? process_scheduled_works+0x2f/0x2f
> [ 1852.759470] [<ffffffff8108e079>] kthread+0xc9/0xd1
> [ 1852.843099] [<ffffffff8109427f>] ? finish_task_switch+0x3a/0xce
> [ 1852.926518] [<ffffffff8108dfb0>] ? __kthread_parkme+0x61/0x61
> [ 1853.008565] [<ffffffff819cbeac>] ret_from_fork+0x7c/0xb0
> [ 1853.076477] [<ffffffff8108dfb0>] ? __kthread_parkme+0x61/0x61
> [ 1853.140653] ---[ end trace 69c4c6617f78e32d ]---
>
> It looks wrong that we add "/proc/net/nfsfs" in nfs_fs_proc_net_init()
> while remove "/proc/fs/nfsfs" in nfs_fs_proc_net_exit().
>
> Fixes: commit 65b38851a17 (NFS: Fix /proc/fs/nfsfs/servers and /proc/fs/nfsfs/volumes)
> Cc: Eric W. Biederman <[email protected]>
> Cc: Trond Myklebust <[email protected]>
> Cc: Dan Aloni <[email protected]>
> Signed-off-by: Cong Wang <[email protected]>
> ---
> v2 - fix nfs_fs_proc_net_init() as well
>
> fs/nfs/client.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 1c5ff6d..c117b96 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -1418,7 +1418,7 @@ int nfs_fs_proc_net_init(struct net *net)
> error_2:
> remove_proc_entry("servers", nn->proc_nfsfs);
> error_1:
> - remove_proc_entry("fs/nfsfs", NULL);
> + remove_proc_entry("nfsfs", net->proc_net);
> error_0:
> return -ENOMEM;
> }
> @@ -1429,7 +1429,7 @@ void nfs_fs_proc_net_exit(struct net *net)
>
> remove_proc_entry("volumes", nn->proc_nfsfs);
> remove_proc_entry("servers", nn->proc_nfsfs);
> - remove_proc_entry("fs/nfsfs", NULL);
> + remove_proc_entry("nfsfs", net->proc_net);
> }
>
> /*
On Tue, Aug 19, 2014 at 09:20:38PM -0700, Eric W. Biederman wrote:
> Cong Wang <[email protected]> writes:
>
> > I saw the following kernel warning:
>
> Cong thanks for finding and tracking this. I was clearly asleep at the
> switch when I was testing my fix to the nfs client code :(
>
> I have applied this patch and will push it to Linus after it has a
> little bit to sit in linux-next.
Why does that code wank with one-by-one remove_proc_entry(), BTW?
remove_proc_subtree("nfsfs", net->proc_net) will take care of the whole pile
just fine, TYVM... While we are it, there's no need to keep ->proc_nfsfs
at all - just have it in a local variable in nfs_fs_proc_net_init().
On Wed, Aug 20, 2014 at 1:24 AM, Christian Kujau <[email protected]> wrote:
> On Tue, 19 Aug 2014 at 21:20, Eric W. Biederman wrote:
>
>> Cong Wang <[email protected]> writes:
>>
>> > I saw the following kernel warning:
>>
>> Cong thanks for finding and tracking this. I was clearly asleep at the
>> switch when I was testing my fix to the nfs client code :(
>>
>> I have applied this patch and will push it to Linus after it has a
>> little bit to sit in linux-next.
>
> This made the boot warning go away for me as well:
> http://lkml.iu.edu/hypermail/linux/kernel/1408.2/01792.html
>
> Tested-by: Christian Kujau <[email protected]>
>
> Thanks!
> Christian.
> --
> BOFH excuse #182:
>
> endothermal recalibration
It is customary (and expected!) to send these patches through the
maintainer; not to push them yourself.
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]
On Mon, Sep 8, 2014 at 4:54 PM, Trond Myklebust
<[email protected]> wrote:
> On Wed, Aug 27, 2014 at 6:41 PM, Al Viro <[email protected]> wrote:
>> On Tue, Aug 19, 2014 at 09:20:38PM -0700, Eric W. Biederman wrote:
>>> Cong Wang <[email protected]> writes:
>>>
>>> > I saw the following kernel warning:
>>>
>>> Cong thanks for finding and tracking this. I was clearly asleep at the
>>> switch when I was testing my fix to the nfs client code :(
>>>
>>> I have applied this patch and will push it to Linus after it has a
>>> little bit to sit in linux-next.
>>
>> Why does that code wank with one-by-one remove_proc_entry(), BTW?
>> remove_proc_subtree("nfsfs", net->proc_net) will take care of the whole pile
>> just fine, TYVM... While we are it, there's no need to keep ->proc_nfsfs
>> at all - just have it in a local variable in nfs_fs_proc_net_init().
>
> Since nobody sent me an updated version with the remove_proc_subtree
> fix, I went ahead and edited the patch myself (see attachment). Cong,
> please let me know if you disagree with that change, otherwise, that
> will be the final patch sent upstream and Cc: stable # 3.4+.
>
> I'll schedule cleanup patches to make the same changes to the original
> nfs_fs_proc_exit() and nfs_fs_proc_init() and to remove (struct
> nfs_net)->proc_nfsfs for merging in 3.18.
>
Oops, I missed Al's reply and didn't know remove_proc_subtree() either.
Thanks for the update and it definitely looks good to me!
On Mon, Sep 8, 2014 at 11:50 AM, Matt Mullins <[email protected]> wrote:
> On Tue, Aug 19, 2014 at 09:20:38PM -0700, Eric W. Biederman wrote:
>> I have applied this patch and will push it to Linus after it has a
>> little bit to sit in linux-next.
what's the status of this patch?
It's rc4 already and I keep seeing it every boot:
[ 7.730034] WARNING: CPU: 3 PID: 103 at ../fs/proc/generic.c:521
remove_proc_entry+0x1a4/0x1b0()
[ 7.730036] remove_proc_entry: removing non-empty directory
'fs/nfsfs', leaking at least 'volumes'
[ 7.730037] Modules linked in: ...
[ 7.730063] CPU: 3 PID: 103 Comm: kworker/u8:15 Not tainted 3.17.0-rc4+ #196
On Wed, Aug 27, 2014 at 6:41 PM, Al Viro <[email protected]> wrote:
> On Tue, Aug 19, 2014 at 09:20:38PM -0700, Eric W. Biederman wrote:
>> Cong Wang <[email protected]> writes:
>>
>> > I saw the following kernel warning:
>>
>> Cong thanks for finding and tracking this. I was clearly asleep at the
>> switch when I was testing my fix to the nfs client code :(
>>
>> I have applied this patch and will push it to Linus after it has a
>> little bit to sit in linux-next.
>
> Why does that code wank with one-by-one remove_proc_entry(), BTW?
> remove_proc_subtree("nfsfs", net->proc_net) will take care of the whole pile
> just fine, TYVM... While we are it, there's no need to keep ->proc_nfsfs
> at all - just have it in a local variable in nfs_fs_proc_net_init().
Since nobody sent me an updated version with the remove_proc_subtree
fix, I went ahead and edited the patch myself (see attachment). Cong,
please let me know if you disagree with that change, otherwise, that
will be the final patch sent upstream and Cc: stable # 3.4+.
I'll schedule cleanup patches to make the same changes to the original
nfs_fs_proc_exit() and nfs_fs_proc_init() and to remove (struct
nfs_net)->proc_nfsfs for merging in 3.18.
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]
On Tue, Aug 19, 2014 at 09:20:38PM -0700, Eric W. Biederman wrote:
> I have applied this patch and will push it to Linus after it has a
> little bit to sit in linux-next.
I also fairly reliably hit this warning with trinity, and with Cong's
patch, it seems to be gone.
If it helps get it pushed toward Linus:
Tested-by: Matt Mullins <[email protected]>