Re: [PATCH tip/core/rcu 14/15] rcu/tree: Allocate a page when caller is preemptible

[Paul E. McKenney]

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