On Thu, Oct 01, 2020 at 11:02:20AM +0200, Michal Hocko wrote:
> On Wed 30-09-20 16:21:54, Paul E. McKenney wrote:
[ . . . ]
Hit "send" too soon, apologies...
> > ------------------------------------------------------------------------
> >
> > commit 490b638d7c241ac06cee168ccf8688bb8b872478
> > Author: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > Date: Wed Sep 30 16:16:39 2020 -0700
> >
> > kvfree_rcu(): Switch from kmalloc/kfree to __get_free_page/free_page.
> >
> > The advantages of using kmalloc() and kfree() are a possible small speedup
> > on CONFIG_SLAB=y systems, avoiding the allocation-side cast, and use of
> > more-familiar API members. The advantages of using __get_free_page()
> > and free_page() are a possible reduction in fragmentation and direct
> > access to the buddy allocator.
> >
> > To help settle the question as to which to use, this commit switches
> > from kmalloc() and kfree() to __get_free_page() and free_page().
> >
> > Suggested-by: Michal Hocko <mhocko@xxxxxxxx>
> > Suggested-by: "Uladzislau Rezki (Sony)" <urezki@xxxxxxxxx>
> > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
>
> Yes, looks good to me. I am not entirely sure about the fragmentation
> argument. It really depends on the SL.B allocator internals. The same
> applies for the potential speed up. I would be even surprised if the
> SLAB was faster in average considering it has to use the page allocator
> as well. So to me the primary motivation would be "use the right tool
> for the purpose".
Very well, I will update the commit message, and thank you!
Thanx, Paul
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 2886e81..242f0f0 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -3225,7 +3225,8 @@ static void kfree_rcu_work(struct work_struct *work)
> > bkvhead[i] = NULL;
> > krc_this_cpu_unlock(krcp, flags);
> >
> > - kfree(bkvhead[i]);
> > + if (bkvhead[i])
> > + free_page((unsigned long)bkvhead[i]);
> >
> > cond_resched_tasks_rcu_qs();
> > }
> > @@ -3378,7 +3379,7 @@ add_ptr_to_bulk_krc_lock(struct kfree_rcu_cpu **krcp,
> > bnode = get_cached_bnode(*krcp);
> > if (!bnode && can_alloc_page) {
> > krc_this_cpu_unlock(*krcp, *flags);
> > - bnode = kmalloc(PAGE_SIZE, gfp);
> > + bnode = (struct kvfree_rcu_bulk_data *)__get_free_page(gfp);
> > *krcp = krc_this_cpu_lock(flags);
> > }
> >
>
> --
> Michal Hocko
> SUSE Labs
