| Message ID | 20220512030442.2530552-6-joel@joelfernandes.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Implement call_rcu_lazy() and miscellaneous fixes | expand |
On Thu, May 12, 2022 at 03:04:33AM +0000, Joel Fernandes (Google) 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). > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> Some of these callbacks do additional work. I don't immediately see a problem, but careful and thorough testing is required. Thanx, Paul > --- > fs/dcache.c | 4 ++-- > fs/eventpoll.c | 2 +- > fs/file_table.c | 3 ++- > fs/inode.c | 2 +- > 4 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/fs/dcache.c b/fs/dcache.c > index c84269c6e8bf..517e02cde103 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 e2daa940ebce..10a24cca2cff 100644 > --- a/fs/eventpoll.c > +++ b/fs/eventpoll.c > @@ -728,7 +728,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 7d2e692b66a9..415815d3ef80 100644 > --- a/fs/file_table.c > +++ b/fs/file_table.c > @@ -56,7 +56,8 @@ 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 63324df6fa27..b288a5bef4c7 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.36.0.550.gb090851708-goog >
On Thu, May 12, 2022 at 05:07:17PM -0700, Paul E. McKenney wrote: > On Thu, May 12, 2022 at 03:04:33AM +0000, Joel Fernandes (Google) 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). > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > > Some of these callbacks do additional work. I don't immediately see a > problem, but careful and thorough testing is required. Full agreed, fwiw I did choose several of them by looking at what the callback did but I agree additional review and testing is needed. thanks, - Joel > > Thanx, Paul > > > --- > > fs/dcache.c | 4 ++-- > > fs/eventpoll.c | 2 +- > > fs/file_table.c | 3 ++- > > fs/inode.c | 2 +- > > 4 files changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/fs/dcache.c b/fs/dcache.c > > index c84269c6e8bf..517e02cde103 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 e2daa940ebce..10a24cca2cff 100644 > > --- a/fs/eventpoll.c > > +++ b/fs/eventpoll.c > > @@ -728,7 +728,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 7d2e692b66a9..415815d3ef80 100644 > > --- a/fs/file_table.c > > +++ b/fs/file_table.c > > @@ -56,7 +56,8 @@ 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 63324df6fa27..b288a5bef4c7 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.36.0.550.gb090851708-goog > >
diff --git a/fs/dcache.c b/fs/dcache.c index c84269c6e8bf..517e02cde103 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 e2daa940ebce..10a24cca2cff 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -728,7 +728,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 7d2e692b66a9..415815d3ef80 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -56,7 +56,8 @@ 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 63324df6fa27..b288a5bef4c7 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); } /**
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). Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> --- fs/dcache.c | 4 ++-- fs/eventpoll.c | 2 +- fs/file_table.c | 3 ++- fs/inode.c | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-)