* Re: discussing about proc_misc_d_delete [not found] <CAPmgiUJVaACDyWkEhpC5Tfk233t-Tw6_f-Y99KLUDqv6dEq0tw@mail.gmail.com> @ 2022-03-17 9:54 ` Alexey Dobriyan 2022-03-18 6:51 ` hui li 0 siblings, 1 reply; 6+ messages in thread From: Alexey Dobriyan @ 2022-03-17 9:54 UTC (permalink / raw) To: hui li; +Cc: viro, linux-kernel [cc linux-kernel ] On Mon, Mar 14, 2022 at 11:54:37AM +0800, hui li wrote: > We noticed that, commit 1da4d377f94 (“proc: revalidate misc dentries”) > introduced proc_misc_dentry_ops as default ops for /proc dentry, > dentry ops for /proc/pid/net/stat/ is set as proc_net_dentry_ops, > which will revalidate dentry each time when this path is resolved and > dentry for the stat file is removed from dcache. This time, if files > under /proc/pid/net/stat/ are in use, then dentries of these files > will be put in lru when closed, which is meanlingless, as parrent > dentry (stat) of these files are remove from dcache. > > This can be reproduced when use linux command "while :;do du > /proc/;done”, then refcount of each dentry of /proc/pid/net/stat/ will > increase rapidly which should be deleted at once. Are you worried that reference count can overflow? Those dentries will be flushed eventually and reference count goes back to normal values. This is easy to see with "echo 3 >/proc/sys/vm/drop_caches". > I think this problem may by solved by checking whether parrent > dentries are in d_cache inside proc_misc_d_delete, or set > proc_misc_dentry_ops->d_delete = always_delete_dentry, just as what is > used in kernel version 4.x and 3.x. > --- a/fs/proc/generic.c > +++ b/fs/proc/generic.c > @@ -236,6 +236,16 @@ static int proc_misc_d_revalidate(struct dentry > *dentry, unsigned int flags) > > static int proc_misc_d_delete(const struct dentry *dentry) > { > + struct dentry *p; > + for (p = dentry->d_parent; !IS_ROOT(p); p = p->d_parent) { > + if (!spin_trylock(&p->d_lock)) > + break; > + if (unlikely(d_unhashed(p))){ > + spin_unlock(&p->d_lock); > + return 1; > + } > + spin_unlock(&p->d_lock); > + } > return atomic_read(&PDE(d_inode(dentry))->in_use) < 0; > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: discussing about proc_misc_d_delete 2022-03-17 9:54 ` discussing about proc_misc_d_delete Alexey Dobriyan @ 2022-03-18 6:51 ` hui li 2022-03-20 16:23 ` [PATCH] proc: fix dentry/inode overinstantiating under /proc/${pid}/net Alexey Dobriyan 0 siblings, 1 reply; 6+ messages in thread From: hui li @ 2022-03-18 6:51 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: viro, linux-kernel Yes, there may be overflow problems when increased very rapidly (refcout of /proc/pid/net/). Dentries are put on lru as they may be accessed again in the future, these dentries will never be accessible for the system, keeping them on lru is a waste of memory, releasing them may be a better choice. In the production environment, we see more than fifty million proc_inode_cache, using "echo 3 >/proc/sys/vm/drop_caches" takes long time and may cause performance problems as drop cache is a heavy work. Besides, we believe that in function drop_pagecache_sb there is a chance that s_inode_list_lock may be held for long time as "inode->i_mapping->nrpages == 0" is always true which may defer inode creation and deletion under /proc when there are too much proc inode caches. Alexey Dobriyan <adobriyan@gmail.com> 于2022年3月17日周四 17:54写道： > > [cc linux-kernel ] > > On Mon, Mar 14, 2022 at 11:54:37AM +0800, hui li wrote: > > We noticed that, commit 1da4d377f94 (“proc: revalidate misc dentries”) > > introduced proc_misc_dentry_ops as default ops for /proc dentry, > > dentry ops for /proc/pid/net/stat/ is set as proc_net_dentry_ops, > > which will revalidate dentry each time when this path is resolved and > > dentry for the stat file is removed from dcache. This time, if files > > under /proc/pid/net/stat/ are in use, then dentries of these files > > will be put in lru when closed, which is meanlingless, as parrent > > dentry (stat) of these files are remove from dcache. > > > > This can be reproduced when use linux command "while :;do du > > /proc/;done”, then refcount of each dentry of /proc/pid/net/stat/ will > > increase rapidly which should be deleted at once. > > Are you worried that reference count can overflow? Those dentries will be > flushed eventually and reference count goes back to normal values. > This is easy to see with "echo 3 >/proc/sys/vm/drop_caches". > > > I think this problem may by solved by checking whether parrent > > dentries are in d_cache inside proc_misc_d_delete, or set > > proc_misc_dentry_ops->d_delete = always_delete_dentry, just as what is > > used in kernel version 4.x and 3.x. > > --- a/fs/proc/generic.c > > +++ b/fs/proc/generic.c > > @@ -236,6 +236,16 @@ static int proc_misc_d_revalidate(struct dentry > > *dentry, unsigned int flags) > > > > static int proc_misc_d_delete(const struct dentry *dentry) > > { > > + struct dentry *p; > > + for (p = dentry->d_parent; !IS_ROOT(p); p = p->d_parent) { > > + if (!spin_trylock(&p->d_lock)) > > + break; > > + if (unlikely(d_unhashed(p))){ > > + spin_unlock(&p->d_lock); > > + return 1; > > + } > > + spin_unlock(&p->d_lock); > > + } > > return atomic_read(&PDE(d_inode(dentry))->in_use) < 0; > > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] proc: fix dentry/inode overinstantiating under /proc/${pid}/net 2022-03-18 6:51 ` hui li @ 2022-03-20 16:23 ` Alexey Dobriyan 2022-03-21 9:15 ` hui li 0 siblings, 1 reply; 6+ messages in thread From: Alexey Dobriyan @ 2022-03-20 16:23 UTC (permalink / raw) To: akpm; +Cc: viro, linux-kernel, hui li, linux-fsdevel When a process exits, /proc/${pid}, and /proc/${pid}/net dentries are flushed. However some leaf dentries like /proc/${pid}/net/arp_cache aren't. That's because respective PDEs have proc_misc_d_revalidate() hook which returns 1 and leaves dentries/inodes in the LRU. Force revalidation/lookup on everything under /proc/${pid}/net by inheriting proc_net_dentry_ops. Fixes: c6c75deda813 ("proc: fix lookup in /proc/net subdirectories after setns(2)") Reported-by: hui li <juanfengpy@gmail.com> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> --- fs/proc/generic.c | 4 ++++ fs/proc/proc_net.c | 3 +++ 2 files changed, 7 insertions(+) --- a/fs/proc/generic.c +++ b/fs/proc/generic.c @@ -448,6 +448,10 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent, proc_set_user(ent, (*parent)->uid, (*parent)->gid); ent->proc_dops = &proc_misc_dentry_ops; + /* Revalidate everything under /proc/${pid}/net */ + if ((*parent)->proc_dops == &proc_net_dentry_ops) { + pde_force_lookup(ent); + } out: return ent; --- a/fs/proc/proc_net.c +++ b/fs/proc/proc_net.c @@ -376,6 +376,9 @@ static __net_init int proc_net_ns_init(struct net *net) proc_set_user(netd, uid, gid); + /* Seed dentry revalidation for /proc/${pid}/net */ + pde_force_lookup(netd); + err = -EEXIST; net_statd = proc_net_mkdir(net, "stat", netd); if (!net_statd) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] proc: fix dentry/inode overinstantiating under /proc/${pid}/net 2022-03-20 16:23 ` [PATCH] proc: fix dentry/inode overinstantiating under /proc/${pid}/net Alexey Dobriyan @ 2022-03-21 9:15 ` hui li 2022-03-21 10:40 ` Alexey Dobriyan 0 siblings, 1 reply; 6+ messages in thread From: hui li @ 2022-03-21 9:15 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: akpm, viro, linux-kernel, linux-fsdevel proc_misc_dentry_ops is a general ops for dentry under /proc, except for "/proc/${pid}/net"，other dentries may also use there own ops too, so I think change proc_misc_d_delete may be better? see patch under: https://lkml.org/lkml/2022/3/17/319 Alexey Dobriyan <adobriyan@gmail.com> 于2022年3月21日周一 00:24写道： > > When a process exits, /proc/${pid}, and /proc/${pid}/net dentries are flushed. > However some leaf dentries like /proc/${pid}/net/arp_cache aren't. > That's because respective PDEs have proc_misc_d_revalidate() hook which > returns 1 and leaves dentries/inodes in the LRU. > > Force revalidation/lookup on everything under /proc/${pid}/net by inheriting > proc_net_dentry_ops. > > Fixes: c6c75deda813 ("proc: fix lookup in /proc/net subdirectories after setns(2)") > Reported-by: hui li <juanfengpy@gmail.com> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> > --- > > fs/proc/generic.c | 4 ++++ > fs/proc/proc_net.c | 3 +++ > 2 files changed, 7 insertions(+) > > --- a/fs/proc/generic.c > +++ b/fs/proc/generic.c > @@ -448,6 +448,10 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent, > proc_set_user(ent, (*parent)->uid, (*parent)->gid); > > ent->proc_dops = &proc_misc_dentry_ops; > + /* Revalidate everything under /proc/${pid}/net */ > + if ((*parent)->proc_dops == &proc_net_dentry_ops) { > + pde_force_lookup(ent); > + } > > out: > return ent; > --- a/fs/proc/proc_net.c > +++ b/fs/proc/proc_net.c > @@ -376,6 +376,9 @@ static __net_init int proc_net_ns_init(struct net *net) > > proc_set_user(netd, uid, gid); > > + /* Seed dentry revalidation for /proc/${pid}/net */ > + pde_force_lookup(netd); > + > err = -EEXIST; > net_statd = proc_net_mkdir(net, "stat", netd); > if (!net_statd) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] proc: fix dentry/inode overinstantiating under /proc/${pid}/net 2022-03-21 9:15 ` hui li @ 2022-03-21 10:40 ` Alexey Dobriyan 2022-03-21 11:50 ` cael 0 siblings, 1 reply; 6+ messages in thread From: Alexey Dobriyan @ 2022-03-21 10:40 UTC (permalink / raw) To: hui li; +Cc: akpm, viro, linux-kernel, linux-fsdevel On Mon, Mar 21, 2022 at 05:15:02PM +0800, hui li wrote: > Alexey Dobriyan <adobriyan@gmail.com> 于2022年3月21日周一 00:24写道： > > > > When a process exits, /proc/${pid}, and /proc/${pid}/net dentries are flushed. > > However some leaf dentries like /proc/${pid}/net/arp_cache aren't. > > That's because respective PDEs have proc_misc_d_revalidate() hook which > > returns 1 and leaves dentries/inodes in the LRU. > > > > Force revalidation/lookup on everything under /proc/${pid}/net by inheriting > > proc_net_dentry_ops. > > > > Fixes: c6c75deda813 ("proc: fix lookup in /proc/net subdirectories after setns(2)") > > Reported-by: hui li <juanfengpy@gmail.com> > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> > > --- > > > > fs/proc/generic.c | 4 ++++ > > fs/proc/proc_net.c | 3 +++ > > 2 files changed, 7 insertions(+) > > > > --- a/fs/proc/generic.c > > +++ b/fs/proc/generic.c > > @@ -448,6 +448,10 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent, > > proc_set_user(ent, (*parent)->uid, (*parent)->gid); > > > > ent->proc_dops = &proc_misc_dentry_ops; > > + /* Revalidate everything under /proc/${pid}/net */ > > + if ((*parent)->proc_dops == &proc_net_dentry_ops) { > > + pde_force_lookup(ent); > > + } > > > > out: > > return ent; > > --- a/fs/proc/proc_net.c > > +++ b/fs/proc/proc_net.c > > @@ -376,6 +376,9 @@ static __net_init int proc_net_ns_init(struct net *net) > > > > proc_set_user(netd, uid, gid); > > > > + /* Seed dentry revalidation for /proc/${pid}/net */ > > + pde_force_lookup(netd); > > + > > err = -EEXIST; > > net_statd = proc_net_mkdir(net, "stat", netd); > > if (!net_statd) > proc_misc_dentry_ops is a general ops for dentry under /proc, except > for "/proc/${pid}/net"，other dentries may also use there own ops too, > so I think change proc_misc_d_delete may be better? > see patch under: https://lkml.org/lkml/2022/3/17/319 I don't think so. proc_misc_d_delete covers "everything else" part under /proc/ and /proc/net which are 2 separate trees. Now /proc/net/ requires revalidation because of commit c6c75deda81344c3a95d1d1f606d5cee109e5d54 proc: fix lookup in /proc/net subdirectories after setns(2) so the bug is that the above commit was applied only partially. In particular, /proc/*/net/stat/arp_cache was created with proc_create_seq_data(), avoiding proc_net_* APIs. And there is probably the same "lookup after setns find wrong file" if you search hard enough in /proc/*/net/ This is the logic. Please test on your systems. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] proc: fix dentry/inode overinstantiating under /proc/${pid}/net 2022-03-21 10:40 ` Alexey Dobriyan @ 2022-03-21 11:50 ` cael 0 siblings, 0 replies; 6+ messages in thread From: cael @ 2022-03-21 11:50 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: akpm, viro, linux-kernel, linux-fsdevel I see, I will test on my system. Alexey Dobriyan <adobriyan@gmail.com> 于2022年3月21日周一 18:40写道： > > On Mon, Mar 21, 2022 at 05:15:02PM +0800, hui li wrote: > > Alexey Dobriyan <adobriyan@gmail.com> 于2022年3月21日周一 00:24写道： > > > > > > When a process exits, /proc/${pid}, and /proc/${pid}/net dentries are flushed. > > > However some leaf dentries like /proc/${pid}/net/arp_cache aren't. > > > That's because respective PDEs have proc_misc_d_revalidate() hook which > > > returns 1 and leaves dentries/inodes in the LRU. > > > > > > Force revalidation/lookup on everything under /proc/${pid}/net by inheriting > > > proc_net_dentry_ops. > > > > > > Fixes: c6c75deda813 ("proc: fix lookup in /proc/net subdirectories after setns(2)") > > > Reported-by: hui li <juanfengpy@gmail.com> > > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> > > > --- > > > > > > fs/proc/generic.c | 4 ++++ > > > fs/proc/proc_net.c | 3 +++ > > > 2 files changed, 7 insertions(+) > > > > > > --- a/fs/proc/generic.c > > > +++ b/fs/proc/generic.c > > > @@ -448,6 +448,10 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent, > > > proc_set_user(ent, (*parent)->uid, (*parent)->gid); > > > > > > ent->proc_dops = &proc_misc_dentry_ops; > > > + /* Revalidate everything under /proc/${pid}/net */ > > > + if ((*parent)->proc_dops == &proc_net_dentry_ops) { > > > + pde_force_lookup(ent); > > > + } > > > > > > out: > > > return ent; > > > --- a/fs/proc/proc_net.c > > > +++ b/fs/proc/proc_net.c > > > @@ -376,6 +376,9 @@ static __net_init int proc_net_ns_init(struct net *net) > > > > > > proc_set_user(netd, uid, gid); > > > > > > + /* Seed dentry revalidation for /proc/${pid}/net */ > > > + pde_force_lookup(netd); > > > + > > > err = -EEXIST; > > > net_statd = proc_net_mkdir(net, "stat", netd); > > > if (!net_statd) > > > proc_misc_dentry_ops is a general ops for dentry under /proc, except > > for "/proc/${pid}/net"，other dentries may also use there own ops too, > > so I think change proc_misc_d_delete may be better? > > see patch under: https://lkml.org/lkml/2022/3/17/319 > > I don't think so. > > proc_misc_d_delete covers "everything else" part under /proc/ and > /proc/net which are 2 separate trees. Now /proc/net/ requires > revalidation because of > > commit c6c75deda81344c3a95d1d1f606d5cee109e5d54 > proc: fix lookup in /proc/net subdirectories after setns(2) > > so the bug is that the above commit was applied only partially. > In particular, /proc/*/net/stat/arp_cache was created with > proc_create_seq_data(), avoiding proc_net_* APIs. > > And there is probably the same "lookup after setns find wrong file" > if you search hard enough in /proc/*/net/ > > This is the logic. Please test on your systems. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-21 11:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAPmgiUJVaACDyWkEhpC5Tfk233t-Tw6_f-Y99KLUDqv6dEq0tw@mail.gmail.com>
2022-03-17 9:54 ` discussing about proc_misc_d_delete Alexey Dobriyan
2022-03-18 6:51 ` hui li
2022-03-20 16:23 ` [PATCH] proc: fix dentry/inode overinstantiating under /proc/${pid}/net Alexey Dobriyan
2022-03-21 9:15 ` hui li
2022-03-21 10:40 ` Alexey Dobriyan
2022-03-21 11:50 ` cael
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox