| Message ID | 1408217806-25877-1-git-send-email-xiyou.wangcong@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Cong Wang <xiyou.wangcong@gmail.com> 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 <ebiederm@xmission.com> > Cc: Trond Myklebust <trond.myklebust@primarydata.com> > Cc: Dan Aloni <dan@kernelim.com> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> > --- > 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); > } > > /* -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 20, 2014 at 1:24 AM, Christian Kujau <lists@nerdbynature.de> wrote: > On Tue, 19 Aug 2014 at 21:20, Eric W. Biederman wrote: > >> Cong Wang <xiyou.wangcong@gmail.com> 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 <lists@nerdbynature.de> > > Thanks! > Christian. > -- > BOFH excuse #182: > > endothermal recalibration It is customary (and expected!) to send these patches through the maintainer; not to push them yourself.
On Wed, Aug 20, 2014 at 12:20 AM, Eric W. Biederman <ebiederm@xmission.com> wrote: > Cong Wang <xiyou.wangcong@gmail.com> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 19, 2014 at 09:20:38PM -0700, Eric W. Biederman wrote: > Cong Wang <xiyou.wangcong@gmail.com> 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(). -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <mmullins@twopensource.com> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 8, 2014 at 11:50 AM, Matt Mullins <mmullins@twopensource.com> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 27, 2014 at 6:41 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > On Tue, Aug 19, 2014 at 09:20:38PM -0700, Eric W. Biederman wrote: >> Cong Wang <xiyou.wangcong@gmail.com> 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.
On Mon, Sep 8, 2014 at 4:54 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > On Wed, Aug 27, 2014 at 6:41 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: >> On Tue, Aug 19, 2014 at 09:20:38PM -0700, Eric W. Biederman wrote: >>> Cong Wang <xiyou.wangcong@gmail.com> 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! -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); } /*
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 <ebiederm@xmission.com> Cc: Trond Myklebust <trond.myklebust@primarydata.com> Cc: Dan Aloni <dan@kernelim.com> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> --- v2 - fix nfs_fs_proc_net_init() as well fs/nfs/client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html