Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAPmgiUJVaACDyWkEhpC5Tfk233t-Tw6_f-Y99KLUDqv6dEq0tw@mail.gmail.com>
 <YjMFTSKZp9eX/c4k@localhost.localdomain>
In-Reply-To: <YjMFTSKZp9eX/c4k@localhost.localdomain>
From:   hui li <juanfengpy@gmail.com>
Date:   Fri, 18 Mar 2022 14:51:15 +0800
Message-ID: <CAPmgiUJsd-gdq=JG1rF8BHfpADeS45rcVWwnC2qKE=7W1EryiQ@mail.gmail.com>
Subject: Re: discussing about proc_misc_d_delete
To:     Alexey Dobriyan <adobriyan@gmail.com>
Cc:     viro@zeniv.linux.org.uk, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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 =3D=3D 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> =E4=BA=8E2022=E5=B9=B43=E6=9C=8817=E6=
=97=A5=E5=91=A8=E5=9B=9B 17:54=E5=86=99=E9=81=93=EF=BC=9A
>
>         [cc linux-kernel ]
>
> On Mon, Mar 14, 2022 at 11:54:37AM +0800, hui li wrote:
> > We noticed that, commit 1da4d377f94 (=E2=80=9Cproc: revalidate misc den=
tries=E2=80=9D)
> > 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=E2=80=9D, then refcount of each dentry of /proc/pid/net/sta=
t/ 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 =3D always_delete_dentry, just as what i=
s
> > 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 =3D dentry->d_parent; !IS_ROOT(p); p =3D 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;
> >  }