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: <20220809034517.3867176-1-joel@joelfernandes.org> <20220809034517.3867176-5-joel@joelfernandes.org>
In-Reply-To: <20220809034517.3867176-5-joel@joelfernandes.org>
From:   Joel Fernandes <joel@joelfernandes.org>
Date:   Thu, 18 Aug 2022 13:22:20 -0400
Message-ID: <CAEXW_YSMYj6DN+ps09SkU2t1hNNg+xGf-6c0rL8dBoy262_-ew@mail.gmail.com>
Subject: Re: [PATCH v3 resend 4/6] fs: Move call_rcu() to call_rcu_lazy() in
 some paths
To:     LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Aug 8, 2022 at 11:45 PM Joel Fernandes (Google)
<joel@joelfernandes.org> wrote:
>
> This is required to prevent callbacks triggering RCU machinery too
> quickly and too often, which adds more power to the system.
>
> When testing, we found that these paths were invoked often when the
> system is not doing anything (screen is ON but otherwise idle).

Unfortunately, I am seeing a slow down in ChromeOS boot performance
after applying this particular patch. It is the first time I could
test ChromeOS boot times with the series since it was hard to find a
ChromeOS device that runs the upstream kernel.

Anyway, Vlad, Neeraj, do you guys also see slower boot times with this
patch? I wonder if the issue is with wake up interaction with the nocb
GP threads.

We ought to disable lazy RCU during boot since it would have little
benefit anyway. But I am also concerned about some deeper problem I
did not catch before.

I'll look into tracing the fs paths to see if I can narrow down what's
causing it. Will also try a newer kernel, I am currently testing on
5.19-rc4.

Thanks,

 - Joel

>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
>  fs/dcache.c     | 4 ++--
>  fs/eventpoll.c  | 2 +-
>  fs/file_table.c | 2 +-
>  fs/inode.c      | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 93f4f5ee07bf..7f51bac390c8 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -366,7 +366,7 @@ static void dentry_free(struct dentry *dentry)
>         if (unlikely(dname_external(dentry))) {
>                 struct external_name *p = external_name(dentry);
>                 if (likely(atomic_dec_and_test(&p->u.count))) {
> -                       call_rcu(&dentry->d_u.d_rcu, __d_free_external);
> +                       call_rcu_lazy(&dentry->d_u.d_rcu, __d_free_external);
>                         return;
>                 }
>         }
> @@ -374,7 +374,7 @@ static void dentry_free(struct dentry *dentry)
>         if (dentry->d_flags & DCACHE_NORCU)
>                 __d_free(&dentry->d_u.d_rcu);
>         else
> -               call_rcu(&dentry->d_u.d_rcu, __d_free);
> +               call_rcu_lazy(&dentry->d_u.d_rcu, __d_free);
>  }
>
>  /*
> diff --git a/fs/eventpoll.c b/fs/eventpoll.c
> index 971f98af48ff..57b3f781760c 100644
> --- a/fs/eventpoll.c
> +++ b/fs/eventpoll.c
> @@ -729,7 +729,7 @@ static int ep_remove(struct eventpoll *ep, struct epitem *epi)
>          * ep->mtx. The rcu read side, reverse_path_check_proc(), does not make
>          * use of the rbn field.
>          */
> -       call_rcu(&epi->rcu, epi_rcu_free);
> +       call_rcu_lazy(&epi->rcu, epi_rcu_free);
>
>         percpu_counter_dec(&ep->user->epoll_watches);
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 5424e3a8df5f..417f57e9cb30 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -56,7 +56,7 @@ static inline void file_free(struct file *f)
>         security_file_free(f);
>         if (!(f->f_mode & FMODE_NOACCOUNT))
>                 percpu_counter_dec(&nr_files);
> -       call_rcu(&f->f_u.fu_rcuhead, file_free_rcu);
> +       call_rcu_lazy(&f->f_u.fu_rcuhead, file_free_rcu);
>  }
>
>  /*
> diff --git a/fs/inode.c b/fs/inode.c
> index bd4da9c5207e..38fe040ddbd6 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -312,7 +312,7 @@ static void destroy_inode(struct inode *inode)
>                         return;
>         }
>         inode->free_inode = ops->free_inode;
> -       call_rcu(&inode->i_rcu, i_callback);
> +       call_rcu_lazy(&inode->i_rcu, i_callback);
>  }
>
>  /**
> --
> 2.37.1.559.g78731f0fdb-goog
>