Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

On Mon, Jun 22, 2020 at 12:53:29PM -0700, Paul E. McKenney wrote:
> On Mon, Jun 22, 2020 at 09:04:06PM +0200, Uladzislau Rezki wrote:
> > > > > 
> > > > > Very good.  When does kfree_rcu() and friends move out of kernel/rcu?
> > > > > 
> > > > Do you mean to move the whole logic of kfree_rcu() from top to down to 
> > > > mm/?
> > > 
> > > I do mean exactly that.
> > > 
> > > That was my goal some years back when Rao Shoaib was making the first
> > > attempt along these lines, and it remains my goal.  After all, if this
> > > effort is at all successful, the coupling between kfree_rcu() with
> > > slab/slob/slub will become much tighter than that between kfree_rcu()
> > > and RCU.
> > > 
> > > There will need to be some additional RCU APIs used by kfree_rcu(),
> > > for example, something to tell RCU how many blocks are awaiting a
> > > grace period.  But these are narrow and easily defined APIs.
> > >
> > I also think that k[v]free_rcu() should reside somewhere under "mm/".
> > Currently they are defined as macros under "linux/rcupdate.h". So i
> > am not sure if definitions should stay there or moved also.
> 
> I am not as worried about the high-level macros as I am about the code
> that does the bulk of the work, but they should still move as well.
> Otherwise, changes involving both the macros and the underlying
> implementation are harder than needed.
> 
> > Implementation of the k[v]free_rcu() is under rcu/tree.c and for tiny
> > variant is under rcutiny.h. It can be moved to the mm/slab_common.c
> > or independent files can be created. I think, mm people should consult
> > what is the best way to go :)
> > 
> > Any thoughts on it?
> 
> I don't have any opinion on exactly where in mm the underlying
> implementation code should go.  You suggestion of mm/slab_common.c
> seems fine to me.  ;-)
> 
OK :)

Then i would like to hear an opinion from the "mm" people where
kfree_rcu() and friends could potentially be moved.

Matthew, Michal, Vlastimil could you please share your view?

Thanks!

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Paul E. McKenney]

On Mon, Jun 22, 2020 at 09:04:06PM +0200, Uladzislau Rezki wrote:
> > > > 
> > > > Very good.  When does kfree_rcu() and friends move out of kernel/rcu?
> > > > 
> > > Do you mean to move the whole logic of kfree_rcu() from top to down to 
> > > mm/?
> > 
> > I do mean exactly that.
> > 
> > That was my goal some years back when Rao Shoaib was making the first
> > attempt along these lines, and it remains my goal.  After all, if this
> > effort is at all successful, the coupling between kfree_rcu() with
> > slab/slob/slub will become much tighter than that between kfree_rcu()
> > and RCU.
> > 
> > There will need to be some additional RCU APIs used by kfree_rcu(),
> > for example, something to tell RCU how many blocks are awaiting a
> > grace period.  But these are narrow and easily defined APIs.
> >
> I also think that k[v]free_rcu() should reside somewhere under "mm/".
> Currently they are defined as macros under "linux/rcupdate.h". So i
> am not sure if definitions should stay there or moved also.

I am not as worried about the high-level macros as I am about the code
that does the bulk of the work, but they should still move as well.
Otherwise, changes involving both the macros and the underlying
implementation are harder than needed.

> Implementation of the k[v]free_rcu() is under rcu/tree.c and for tiny
> variant is under rcutiny.h. It can be moved to the mm/slab_common.c
> or independent files can be created. I think, mm people should consult
> what is the best way to go :)
> 
> Any thoughts on it?

I don't have any opinion on exactly where in mm the underlying
implementation code should go.  You suggestion of mm/slab_common.c
seems fine to me.  ;-)

Thanx, Paul

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

> > > 
> > > Very good.  When does kfree_rcu() and friends move out of kernel/rcu?
> > > 
> > Do you mean to move the whole logic of kfree_rcu() from top to down to mm/?
> 
> I do mean exactly that.
> 
> That was my goal some years back when Rao Shoaib was making the first
> attempt along these lines, and it remains my goal.  After all, if this
> effort is at all successful, the coupling between kfree_rcu() with
> slab/slob/slub will become much tighter than that between kfree_rcu()
> and RCU.
> 
> There will need to be some additional RCU APIs used by kfree_rcu(),
> for example, something to tell RCU how many blocks are awaiting a
> grace period.  But these are narrow and easily defined APIs.
>
I also think that k[v]free_rcu() should reside somewhere under "mm/".
Currently they are defined as macros under "linux/rcupdate.h". So i
am not sure if definitions should stay there or moved also.

Implementation of the k[v]free_rcu() is under rcu/tree.c and for tiny
variant is under rcutiny.h. It can be moved to the mm/slab_common.c
or independent files can be created. I think, mm people should consult
what is the best way to go :)

Any thoughts on it?

Thank you!

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Paul E. McKenney]

On Fri, Jun 19, 2020 at 05:46:52PM +0200, Uladzislau Rezki wrote:
> On Thu, Jun 18, 2020 at 02:34:27PM -0700, Paul E. McKenney wrote:
> > On Thu, Jun 18, 2020 at 11:17:09PM +0200, Uladzislau Rezki wrote:
> > > > > 
> > > > > trace_rcu_invoke_kfree_bulk_callback(
> > > > > rcu_state.name, bkvhead[i]->nr_records,
> > > > > bkvhead[i]->records);
> > > > > if (i == 0)
> > > > > kfree_bulk(bkvhead[i]->nr_records,
> > > > > bkvhead[i]->records);
> > > > > else
> > > > > vfree_bulk(bkvhead[i]->nr_records,
> > > > > bkvhead[i]->records);
> > > > > 
> > > > > 
> > > > > Matthew, what is your thought?
> > > > 
> > > > That was my thinking too.  If we had a kvfree_bulk(), I would expect it 
> > > > to
> > > > handle a mixture of vfree and kfree, but you've segregated them already.
> > > > So I think this is better.
> > > >
> > > Yes, the segregation is done. Having vfree_bulk() is enough then.
> > > We are on the same page :)
> > 
> > Very good.  When does kfree_rcu() and friends move out of kernel/rcu?
> > 
> Do you mean to move the whole logic of kfree_rcu() from top to down to mm/?

I do mean exactly that.

That was my goal some years back when Rao Shoaib was making the first
attempt along these lines, and it remains my goal.  After all, if this
effort is at all successful, the coupling between kfree_rcu() with
slab/slob/slub will become much tighter than that between kfree_rcu()
and RCU.

There will need to be some additional RCU APIs used by kfree_rcu(),
for example, something to tell RCU how many blocks are awaiting a
grace period.  But these are narrow and easily defined APIs.

Thanx, Paul

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

On Thu, Jun 18, 2020 at 02:34:27PM -0700, Paul E. McKenney wrote:
> On Thu, Jun 18, 2020 at 11:17:09PM +0200, Uladzislau Rezki wrote:
> > > > 
> > > > trace_rcu_invoke_kfree_bulk_callback(
> > > > rcu_state.name, bkvhead[i]->nr_records,
> > > > bkvhead[i]->records);
> > > > if (i == 0)
> > > > kfree_bulk(bkvhead[i]->nr_records,
> > > > bkvhead[i]->records);
> > > > else
> > > > vfree_bulk(bkvhead[i]->nr_records,
> > > > bkvhead[i]->records);
> > > > 
> > > > 
> > > > Matthew, what is your thought?
> > > 
> > > That was my thinking too.  If we had a kvfree_bulk(), I would expect it to
> > > handle a mixture of vfree and kfree, but you've segregated them already.
> > > So I think this is better.
> > >
> > Yes, the segregation is done. Having vfree_bulk() is enough then.
> > We are on the same page :)
> 
> Very good.  When does kfree_rcu() and friends move out of kernel/rcu?
> 
Do you mean to move the whole logic of kfree_rcu() from top to down to mm/?

Thanks!

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Paul E. McKenney]

On Thu, Jun 18, 2020 at 11:17:09PM +0200, Uladzislau Rezki wrote:
> > > 
> > > trace_rcu_invoke_kfree_bulk_callback(
> > > rcu_state.name, bkvhead[i]->nr_records,
> > > bkvhead[i]->records);
> > > if (i == 0)
> > > kfree_bulk(bkvhead[i]->nr_records,
> > > bkvhead[i]->records);
> > > else
> > > vfree_bulk(bkvhead[i]->nr_records,
> > > bkvhead[i]->records);
> > > 
> > > 
> > > Matthew, what is your thought?
> > 
> > That was my thinking too.  If we had a kvfree_bulk(), I would expect it to
> > handle a mixture of vfree and kfree, but you've segregated them already.
> > So I think this is better.
> >
> Yes, the segregation is done. Having vfree_bulk() is enough then.
> We are on the same page :)

Very good.  When does kfree_rcu() and friends move out of kernel/rcu?

Thanx, Paul

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

> > 
> > trace_rcu_invoke_kfree_bulk_callback(
> > rcu_state.name, bkvhead[i]->nr_records,
> > bkvhead[i]->records);
> > if (i == 0)
> > kfree_bulk(bkvhead[i]->nr_records,
> > bkvhead[i]->records);
> > else
> > vfree_bulk(bkvhead[i]->nr_records,
> > bkvhead[i]->records);
> > 
> > 
> > Matthew, what is your thought?
> 
> That was my thinking too.  If we had a kvfree_bulk(), I would expect it to
> handle a mixture of vfree and kfree, but you've segregated them already.
> So I think this is better.
>
Yes, the segregation is done. Having vfree_bulk() is enough then.
We are on the same page :)

Thanks!

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Matthew Wilcox]

On Thu, Jun 18, 2020 at 10:35:57PM +0200, Uladzislau Rezki wrote:
> On Thu, Jun 18, 2020 at 12:03:59PM -0700, Paul E. McKenney wrote:
> but i do not have a strong opinion here, even though i tend to
> say that it would be odd. Having just vfree_bulk(), i think
> would be enough, as a result the code will look like:
> 
> 
> trace_rcu_invoke_kfree_bulk_callback(
> rcu_state.name, bkvhead[i]->nr_records,
> bkvhead[i]->records);
> if (i == 0)
> kfree_bulk(bkvhead[i]->nr_records,
> bkvhead[i]->records);
> else
> vfree_bulk(bkvhead[i]->nr_records,
> bkvhead[i]->records);
> 
> 
> Matthew, what is your thought?

That was my thinking too.  If we had a kvfree_bulk(), I would expect it to
handle a mixture of vfree and kfree, but you've segregated them already.
So I think this is better.

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

On Thu, Jun 18, 2020 at 12:03:59PM -0700, Paul E. McKenney wrote:
> On Thu, Jun 18, 2020 at 08:34:48PM +0200, Uladzislau Rezki wrote:
> > > > > 
> > > > > I suspect that he would like to keep the tracing.
> > > > > 
> > > > > It might be worth trying the branches, given that they would be 
> > > > > constant
> > > > > and indexed by "i".  The compiler might well remove the indirection.
> > > > > 
> > > > > The compiler guys brag about doing so, which of course might or might
> > > > > not have any correlation to a given compiler actually doing so.  :-/
> > > > > 
> > > > > Having a vfree_bulk() might well be useful, but I would feel more
> > > > > confidence in that if there were other callers of kfree_bulk().
> > > > >
> > > > Hmm... I think replacing that with vfree_bulk() is a good idea though.
> > > 
> > > In other words, get rid of kfree_bulk() in favor of vfree_bulk()?
> > > 
> > kfree_bulk() does not understand vmalloc memory. vfree_bulk() should
> > be implemented to release vmalloc's pointers. On i high level it will
> > be used the same way as kfree_bulk() but for vmalloc ptrs. only.
> 
> Ah, I thought that you guys were proposing something that did bulk
> free of both kmalloc and vmalloc memory.
> 
I see your point. We could introduce something like:

kvfree_bulk(slab_arra, vmalloc_array);

but i do not have a strong opinion here, even though i tend to
say that it would be odd. Having just vfree_bulk(), i think
would be enough, as a result the code will look like:


trace_rcu_invoke_kfree_bulk_callback(
rcu_state.name, bkvhead[i]->nr_records,
bkvhead[i]->records);
if (i == 0)
kfree_bulk(bkvhead[i]->nr_records,
bkvhead[i]->records);
else
vfree_bulk(bkvhead[i]->nr_records,
bkvhead[i]->records);


Matthew, what is your thought?

Thanks!

--
Vlad rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

On Thu, Jun 18, 2020 at 10:35:27AM -0700, Matthew Wilcox wrote:
> On Thu, Jun 18, 2020 at 07:30:49PM +0200, Uladzislau Rezki wrote:
> > > I'd suggest:
> > > 
> > >   rcu_lock_acquire(&rcu_callback_map);
> > >   trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
> > >   bkvhead[i]->nr_records, bkvhead[i]->records);
> > >   if (i == 0) {
> > >   kfree_bulk(bkvhead[i]->nr_records,
> > >   bkvhead[i]->records);
> > >   } else {
> > >   for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > >   vfree(bkvhead[i]->records[j]);
> > >   }
> > >   }
> > >   rcu_lock_release(&rcu_callback_map);
> > >
> > There are two different trace functions, one for "bulk" tracing
> > messages, and another one is per one call of kfree(), though we use 
> > to indicate vfree() call.
> > 
> > Probably we can rename it to: trace_rcu_invoke_kvfree_callback();
> > 
> > What do you think?
> 
> Works for me!
> 
OK. I will send out the patch that will rename that trace function
that makes clear that the pointer that is freed can belong to SLAB
or vmalloc.

> > > But I'd also suggest a vfree_bulk be added.  There are a few things
> > > which would be better done in bulk as part of the vfree process
> > > (we batch them up already, but i'm sure we could do better).
> > 
> > I was thinking to implement of vfree_bulk() API, but i guess it can
> > be done as future work.
> > 
> > Does that sound good?
> 
> Yes, definitely a future piece of work.
>
You have already been doing it.

Thank you, Matthew :)

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Paul E. McKenney]

On Thu, Jun 18, 2020 at 08:34:48PM +0200, Uladzislau Rezki wrote:
> > > > 
> > > > I suspect that he would like to keep the tracing.
> > > > 
> > > > It might be worth trying the branches, given that they would be constant
> > > > and indexed by "i".  The compiler might well remove the indirection.
> > > > 
> > > > The compiler guys brag about doing so, which of course might or might
> > > > not have any correlation to a given compiler actually doing so.  :-/
> > > > 
> > > > Having a vfree_bulk() might well be useful, but I would feel more
> > > > confidence in that if there were other callers of kfree_bulk().
> > > >
> > > Hmm... I think replacing that with vfree_bulk() is a good idea though.
> > 
> > In other words, get rid of kfree_bulk() in favor of vfree_bulk()?
> > 
> kfree_bulk() does not understand vmalloc memory. vfree_bulk() should
> be implemented to release vmalloc's pointers. On i high level it will
> be used the same way as kfree_bulk() but for vmalloc ptrs. only.

Ah, I thought that you guys were proposing something that did bulk
free of both kmalloc and vmalloc memory.

Thanx, Paul

> > > > But again, either way, future work as far as this series is concerned.
> > > > 
> > > What do you mean: is concerned?
> > 
> > Apologies for the strange English.  How about this?
> > 
> > "This series is OK as is.  Any comments above did not prevent me from
> > taking these patches, but instead discuss possible future work."
> > 
> That is perfectly clear to me :)
> 
> > > We are planning to implement kfree_rcu() to be integrated directly into
> > > SLAB: SLAB, SLUB, SLOB. So, there are plenty of future work :)
> >
> > And I am glad that this is still the goal.  ;-)
> >
> :)
> 
> --
> Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

On Thu, Jun 18, 2020 at 11:37:51AM -0700, Matthew Wilcox wrote:
> On Thu, Jun 18, 2020 at 08:23:33PM +0200, Uladzislau Rezki wrote:
> > > +void vfree_bulk(size_t count, void **addrs)
> > > +{
> > > + unsigned int i;
> > > +
> > > + BUG_ON(in_nmi());
> > > + might_sleep_if(!in_interrupt());
> > > +
> > > + for (i = 0; i < count; i++) {
> > > + void *addr = addrs[i];
> > > + kmemleak_free(addr);
> > > + if (addr)
> > > + __vfree(addr);
> > > + }
> > > +}
> > > +EXPORT_SYMBOL(vfree_bulk);
> > > +
> > >
> > Can we just do addrs[i] all over the loop?
> > 
> > Also, we can just call vfree() instead that has all checking we
> > need: NMI, kmemleak, might_sleep.
> 
> Of course we _can_.  But would we want to?  This way, we only do these
> checks once instead of once per pointer, which is rather the point
> of batching.
>
Ahh, right. I briefly looked at it and missed that point. Right you
are we do not want the vfree() here!

> 
> I might actually go further and hoist the in_interrupt() check into
> this function ...
>
Why do you need it? Just to inline below code:


 if (unlikely(in_interrupt()))
  __vfree_deferred(addr);
 else
  __vunmap(addr, 1);


and bypass the __vfree() call(that is not marked as inline one)?
I mean to inline above into  vfree_bulk().

>
> I suspect the RCU code always runs in_interrupt()
> and so we always call vfree_deferred().
>
No. We release the memory from workqueue context.

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Matthew Wilcox]

On Thu, Jun 18, 2020 at 08:23:33PM +0200, Uladzislau Rezki wrote:
> > +void vfree_bulk(size_t count, void **addrs)
> > +{
> > +   unsigned int i;
> > +
> > +   BUG_ON(in_nmi());
> > +   might_sleep_if(!in_interrupt());
> > +
> > +   for (i = 0; i < count; i++) {
> > +   void *addr = addrs[i];
> > +   kmemleak_free(addr);
> > +   if (addr)
> > +   __vfree(addr);
> > +   }
> > +}
> > +EXPORT_SYMBOL(vfree_bulk);
> > +
> >
> Can we just do addrs[i] all over the loop?
> 
> Also, we can just call vfree() instead that has all checking we
> need: NMI, kmemleak, might_sleep.

Of course we _can_.  But would we want to?  This way, we only do these
checks once instead of once per pointer, which is rather the point
of batching.

I might actually go further and hoist the in_interrupt() check into
this function ... I suspect the RCU code always runs in_interrupt()
and so we always call vfree_deferred().

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

> > > 
> > > I suspect that he would like to keep the tracing.
> > > 
> > > It might be worth trying the branches, given that they would be constant
> > > and indexed by "i".  The compiler might well remove the indirection.
> > > 
> > > The compiler guys brag about doing so, which of course might or might
> > > not have any correlation to a given compiler actually doing so.  :-/
> > > 
> > > Having a vfree_bulk() might well be useful, but I would feel more
> > > confidence in that if there were other callers of kfree_bulk().
> > >
> > Hmm... I think replacing that with vfree_bulk() is a good idea though.
> 
> In other words, get rid of kfree_bulk() in favor of vfree_bulk()?
> 
kfree_bulk() does not understand vmalloc memory. vfree_bulk() should
be implemented to release vmalloc's pointers. On i high level it will
be used the same way as kfree_bulk() but for vmalloc ptrs. only.

> > > But again, either way, future work as far as this series is concerned.
> > > 
> > What do you mean: is concerned?
> 
> Apologies for the strange English.  How about this?
> 
> "This series is OK as is.  Any comments above did not prevent me from
> taking these patches, but instead discuss possible future work."
> 
That is perfectly clear to me :)

> > We are planning to implement kfree_rcu() to be integrated directly into
> > SLAB: SLAB, SLUB, SLOB. So, there are plenty of future work :)
>
> And I am glad that this is still the goal.  ;-)
>
:)

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

On Thu, Jun 18, 2020 at 11:15:41AM -0700, Matthew Wilcox wrote:
> On Thu, Jun 18, 2020 at 07:56:23PM +0200, Uladzislau Rezki wrote:
> > If we mix pointers, then we can do free per pointer only. I mean in that
> > case we will not be able to use kfree_bulk() interface for freeing SLAB
> > memory and the code would converted to something like:
> > 
> > 
> > while (nr_objects_in_array > 0) {
> > if (is_vmalloc_addr(array[X]))
> >vfree(array[X]);
> > else
> >kfree(array[X]);
> > }
> > 
> 
> [PATCH] Add vfree_bulk interface
> 
> This is a useful interface to have for the RCU kvfree code.  There is
> scope for more performance gains later, but introducing the interface
> now allows us to simplify the RCU code today.
> 
> Signed-off-by: Matthew Wilcox (Oracle) 
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 48bb681e6c2a..dc2bbb61af61 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -119,6 +119,7 @@ void *__vmalloc_node(unsigned long size, unsigned long 
> align, gfp_t gfp_mask,
>  
>  extern void vfree(const void *addr);
>  extern void vfree_atomic(const void *addr);
> +extern void vfree_bulk(size_t count, void **addrs);
>  
>  extern void *vmap(struct page **pages, unsigned int count,
>   unsigned long flags, pgprot_t prot);
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index abe37f09ac42..6042f9b4394a 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2366,6 +2366,22 @@ void vfree(const void *addr)
>  }
>  EXPORT_SYMBOL(vfree);
>  
> +void vfree_bulk(size_t count, void **addrs)
> +{
> + unsigned int i;
> +
> + BUG_ON(in_nmi());
> + might_sleep_if(!in_interrupt());
> +
> + for (i = 0; i < count; i++) {
> + void *addr = addrs[i];
> + kmemleak_free(addr);
> + if (addr)
> + __vfree(addr);
> + }
> +}
> +EXPORT_SYMBOL(vfree_bulk);
> +
>
Can we just do addrs[i] all over the loop?

Also, we can just call vfree() instead that has all checking we
need: NMI, kmemleak, might_sleep.

Thanks!

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Matthew Wilcox]

On Thu, Jun 18, 2020 at 07:56:23PM +0200, Uladzislau Rezki wrote:
> If we mix pointers, then we can do free per pointer only. I mean in that
> case we will not be able to use kfree_bulk() interface for freeing SLAB
> memory and the code would converted to something like:
> 
> 
> while (nr_objects_in_array > 0) {
> if (is_vmalloc_addr(array[X]))
>vfree(array[X]);
> else
>kfree(array[X]);
> }
> 

[PATCH] Add vfree_bulk interface

This is a useful interface to have for the RCU kvfree code.  There is
scope for more performance gains later, but introducing the interface
now allows us to simplify the RCU code today.

Signed-off-by: Matthew Wilcox (Oracle) 

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 48bb681e6c2a..dc2bbb61af61 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -119,6 +119,7 @@ void *__vmalloc_node(unsigned long size, unsigned long 
align, gfp_t gfp_mask,
 
 extern void vfree(const void *addr);
 extern void vfree_atomic(const void *addr);
+extern void vfree_bulk(size_t count, void **addrs);
 
 extern void *vmap(struct page **pages, unsigned int count,
unsigned long flags, pgprot_t prot);
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index abe37f09ac42..6042f9b4394a 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2366,6 +2366,22 @@ void vfree(const void *addr)
 }
 EXPORT_SYMBOL(vfree);
 
+void vfree_bulk(size_t count, void **addrs)
+{
+   unsigned int i;
+
+   BUG_ON(in_nmi());
+   might_sleep_if(!in_interrupt());
+
+   for (i = 0; i < count; i++) {
+   void *addr = addrs[i];
+   kmemleak_free(addr);
+   if (addr)
+   __vfree(addr);
+   }
+}
+EXPORT_SYMBOL(vfree_bulk);
+
 /**
  * vunmap - release virtual mapping obtained by vmap()
  * @addr:   memory base address

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Paul E. McKenney]

On Thu, Jun 18, 2020 at 07:35:20PM +0200, Uladzislau Rezki wrote:
> > > 
> > > I don't think that replacing direct function calls with indirect function
> > > calls is a great suggestion with the current state of play around branch
> > > prediction.
> > > 
> > > I'd suggest:
> > > 
> > >   rcu_lock_acquire(&rcu_callback_map);
> > >   trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
> > >   bkvhead[i]->nr_records, bkvhead[i]->records);
> > >   if (i == 0) {
> > >   kfree_bulk(bkvhead[i]->nr_records,
> > >   bkvhead[i]->records);
> > >   } else {
> > >   for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > >   vfree(bkvhead[i]->records[j]);
> > >   }
> > >   }
> > >   rcu_lock_release(&rcu_callback_map);
> > > 
> > > But I'd also suggest a vfree_bulk be added.  There are a few things
> > > which would be better done in bulk as part of the vfree process
> > > (we batch them up already, but i'm sure we could do better).
> > 
> > I suspect that he would like to keep the tracing.
> > 
> > It might be worth trying the branches, given that they would be constant
> > and indexed by "i".  The compiler might well remove the indirection.
> > 
> > The compiler guys brag about doing so, which of course might or might
> > not have any correlation to a given compiler actually doing so.  :-/
> > 
> > Having a vfree_bulk() might well be useful, but I would feel more
> > confidence in that if there were other callers of kfree_bulk().
> >
> Hmm... I think replacing that with vfree_bulk() is a good idea though.

In other words, get rid of kfree_bulk() in favor of vfree_bulk()?

> > But again, either way, future work as far as this series is concerned.
> > 
> What do you mean: is concerned?

Apologies for the strange English.  How about this?

"This series is OK as is.  Any comments above did not prevent me from
taking these patches, but instead discuss possible future work."

> We are planning to implement kfree_rcu() to be integrated directly into
> SLAB: SLAB, SLUB, SLOB. So, there are plenty of future work :)

And I am glad that this is still the goal.  ;-)

Thanx, Paul

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

On Thu, Jun 18, 2020 at 10:32:06AM -0700, Paul E. McKenney wrote:
> On Thu, Jun 18, 2020 at 07:25:04PM +0200, Uladzislau Rezki wrote:
> > > > +   // Handle two first channels.
> > > > +   for (i = 0; i < FREE_N_CHANNELS; i++) {
> > > > +   for (; bkvhead[i]; bkvhead[i] = bnext) {
> > > > +   bnext = bkvhead[i]->next;
> > > > +   debug_rcu_bhead_unqueue(bkvhead[i]);
> > > > +
> > > > +   rcu_lock_acquire(&rcu_callback_map);
> > > > +   if (i == 0) { // kmalloc() / kfree().
> > > > +   trace_rcu_invoke_kfree_bulk_callback(
> > > > +   rcu_state.name, 
> > > > bkvhead[i]->nr_records,
> > > > +   bkvhead[i]->records);
> > > > +
> > > > +   kfree_bulk(bkvhead[i]->nr_records,
> > > > +   bkvhead[i]->records);
> > > > +   } else { // vmalloc() / vfree().
> > > > +   for (j = 0; j < bkvhead[i]->nr_records; 
> > > > j++) {
> > > > +   trace_rcu_invoke_kfree_callback(
> > > > +   rcu_state.name,
> > > > +   bkvhead[i]->records[j], 
> > > > 0);
> > > > +
> > > > +   vfree(bkvhead[i]->records[j]);
> > > > +   }
> > > > +   }
> > > > +   rcu_lock_release(&rcu_callback_map);
> > > 
> > > Not an emergency, but did you look into replacing this "if" statement
> > > with an array of pointers to functions implementing the legs of the
> > > "if" statement?  If nothing else, this would greatly reduced indentation.
> > > 
> > >
> > > I am taking this as is, but if you have not already done so, could you
> > > please look into this for a follow-up patch?
> > > 
> > I do not think it makes sense, because it would require to check each
> > pointer in the array, what can lead to many branching, i.e. "if-else"
> > instructions.
> 
> Mightn't the compiler simply unroll the outer loop?  Then the first
> unrolled iteration of that loop would contain the then-clause and
> the second unrolled iteration would contain the else-clause.  At that
> point, there would be no checking, just direct calls.
> 
> Or am I missing something?
> 
If we mix pointers, then we can do free per pointer only. I mean in that
case we will not be able to use kfree_bulk() interface for freeing SLAB
memory and the code would converted to something like:


while (nr_objects_in_array > 0) {
if (is_vmalloc_addr(array[X]))
   vfree(array[X]);
else
   kfree(array[X]);
}


> > Paul, thank you to take it in!
> 
> Thank you for persisting!
> 
Welcome :)

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Matthew Wilcox]

On Thu, Jun 18, 2020 at 07:30:49PM +0200, Uladzislau Rezki wrote:
> > I'd suggest:
> > 
> > rcu_lock_acquire(&rcu_callback_map);
> > trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
> > bkvhead[i]->nr_records, bkvhead[i]->records);
> > if (i == 0) {
> > kfree_bulk(bkvhead[i]->nr_records,
> > bkvhead[i]->records);
> > } else {
> > for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > vfree(bkvhead[i]->records[j]);
> > }
> > }
> > rcu_lock_release(&rcu_callback_map);
> >
> There are two different trace functions, one for "bulk" tracing
> messages, and another one is per one call of kfree(), though we use 
> to indicate vfree() call.
> 
> Probably we can rename it to: trace_rcu_invoke_kvfree_callback();
> 
> What do you think?

Works for me!

> > But I'd also suggest a vfree_bulk be added.  There are a few things
> > which would be better done in bulk as part of the vfree process
> > (we batch them up already, but i'm sure we could do better).
> 
> I was thinking to implement of vfree_bulk() API, but i guess it can
> be done as future work.
> 
> Does that sound good?

Yes, definitely a future piece of work.

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

> > 
> > I don't think that replacing direct function calls with indirect function
> > calls is a great suggestion with the current state of play around branch
> > prediction.
> > 
> > I'd suggest:
> > 
> > rcu_lock_acquire(&rcu_callback_map);
> > trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
> > bkvhead[i]->nr_records, bkvhead[i]->records);
> > if (i == 0) {
> > kfree_bulk(bkvhead[i]->nr_records,
> > bkvhead[i]->records);
> > } else {
> > for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > vfree(bkvhead[i]->records[j]);
> > }
> > }
> > rcu_lock_release(&rcu_callback_map);
> > 
> > But I'd also suggest a vfree_bulk be added.  There are a few things
> > which would be better done in bulk as part of the vfree process
> > (we batch them up already, but i'm sure we could do better).
> 
> I suspect that he would like to keep the tracing.
> 
> It might be worth trying the branches, given that they would be constant
> and indexed by "i".  The compiler might well remove the indirection.
> 
> The compiler guys brag about doing so, which of course might or might
> not have any correlation to a given compiler actually doing so.  :-/
> 
> Having a vfree_bulk() might well be useful, but I would feel more
> confidence in that if there were other callers of kfree_bulk().
>
Hmm... I think replacing that with vfree_bulk() is a good idea though.

> 
> But again, either way, future work as far as this series is concerned.
> 
What do you mean: is concerned?

We are planning to implement kfree_rcu() to be integrated directly into
SLAB: SLAB, SLUB, SLOB. So, there are plenty of future work :)

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Paul E. McKenney]

On Thu, Jun 18, 2020 at 07:25:04PM +0200, Uladzislau Rezki wrote:
> > > + // Handle two first channels.
> > > + for (i = 0; i < FREE_N_CHANNELS; i++) {
> > > + for (; bkvhead[i]; bkvhead[i] = bnext) {
> > > + bnext = bkvhead[i]->next;
> > > + debug_rcu_bhead_unqueue(bkvhead[i]);
> > > +
> > > + rcu_lock_acquire(&rcu_callback_map);
> > > + if (i == 0) { // kmalloc() / kfree().
> > > + trace_rcu_invoke_kfree_bulk_callback(
> > > + rcu_state.name, bkvhead[i]->nr_records,
> > > + bkvhead[i]->records);
> > > +
> > > + kfree_bulk(bkvhead[i]->nr_records,
> > > + bkvhead[i]->records);
> > > + } else { // vmalloc() / vfree().
> > > + for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > > + trace_rcu_invoke_kfree_callback(
> > > + rcu_state.name,
> > > + bkvhead[i]->records[j], 0);
> > > +
> > > + vfree(bkvhead[i]->records[j]);
> > > + }
> > > + }
> > > + rcu_lock_release(&rcu_callback_map);
> > 
> > Not an emergency, but did you look into replacing this "if" statement
> > with an array of pointers to functions implementing the legs of the
> > "if" statement?  If nothing else, this would greatly reduced indentation.
> > 
> >
> > I am taking this as is, but if you have not already done so, could you
> > please look into this for a follow-up patch?
> > 
> I do not think it makes sense, because it would require to check each
> pointer in the array, what can lead to many branching, i.e. "if-else"
> instructions.

Mightn't the compiler simply unroll the outer loop?  Then the first
unrolled iteration of that loop would contain the then-clause and
the second unrolled iteration would contain the else-clause.  At that
point, there would be no checking, just direct calls.

Or am I missing something?

> Paul, thank you to take it in!

Thank you for persisting!

Thanx, Paul

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

> > 
> > Not an emergency, but did you look into replacing this "if" statement
> > with an array of pointers to functions implementing the legs of the
> > "if" statement?  If nothing else, this would greatly reduced indentation.
> 
> I don't think that replacing direct function calls with indirect function
> calls is a great suggestion with the current state of play around branch
> prediction.
> 
> I'd suggest:
> 
>   rcu_lock_acquire(&rcu_callback_map);
>   trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
>   bkvhead[i]->nr_records, bkvhead[i]->records);
>   if (i == 0) {
>   kfree_bulk(bkvhead[i]->nr_records,
>   bkvhead[i]->records);
>   } else {
>   for (j = 0; j < bkvhead[i]->nr_records; j++) {
>   vfree(bkvhead[i]->records[j]);
>   }
>   }
>   rcu_lock_release(&rcu_callback_map);
>
There are two different trace functions, one for "bulk" tracing
messages, and another one is per one call of kfree(), though we use 
to indicate vfree() call.

Probably we can rename it to: trace_rcu_invoke_kvfree_callback();

What do you think?

> 
> But I'd also suggest a vfree_bulk be added.  There are a few things
> which would be better done in bulk as part of the vfree process
> (we batch them up already, but i'm sure we could do better).
> 
I was thinking to implement of vfree_bulk() API, but i guess it can
be done as future work.

Does that sound good?

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki]

> > +   // Handle two first channels.
> > +   for (i = 0; i < FREE_N_CHANNELS; i++) {
> > +   for (; bkvhead[i]; bkvhead[i] = bnext) {
> > +   bnext = bkvhead[i]->next;
> > +   debug_rcu_bhead_unqueue(bkvhead[i]);
> > +
> > +   rcu_lock_acquire(&rcu_callback_map);
> > +   if (i == 0) { // kmalloc() / kfree().
> > +   trace_rcu_invoke_kfree_bulk_callback(
> > +   rcu_state.name, bkvhead[i]->nr_records,
> > +   bkvhead[i]->records);
> > +
> > +   kfree_bulk(bkvhead[i]->nr_records,
> > +   bkvhead[i]->records);
> > +   } else { // vmalloc() / vfree().
> > +   for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > +   trace_rcu_invoke_kfree_callback(
> > +   rcu_state.name,
> > +   bkvhead[i]->records[j], 0);
> > +
> > +   vfree(bkvhead[i]->records[j]);
> > +   }
> > +   }
> > +   rcu_lock_release(&rcu_callback_map);
> 
> Not an emergency, but did you look into replacing this "if" statement
> with an array of pointers to functions implementing the legs of the
> "if" statement?  If nothing else, this would greatly reduced indentation.
> 
>
> I am taking this as is, but if you have not already done so, could you
> please look into this for a follow-up patch?
> 
I do not think it makes sense, because it would require to check each
pointer in the array, what can lead to many branching, i.e. "if-else"
instructions.

Paul, thank you to take it in!

--
Vlad Rezki

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Paul E. McKenney]

On Wed, Jun 17, 2020 at 05:52:14PM -0700, Matthew Wilcox wrote:
> On Wed, Jun 17, 2020 at 04:46:09PM -0700, Paul E. McKenney wrote:
> > > + // Handle two first channels.
> > > + for (i = 0; i < FREE_N_CHANNELS; i++) {
> > > + for (; bkvhead[i]; bkvhead[i] = bnext) {
> > > + bnext = bkvhead[i]->next;
> > > + debug_rcu_bhead_unqueue(bkvhead[i]);
> > > +
> > > + rcu_lock_acquire(&rcu_callback_map);
> > > + if (i == 0) { // kmalloc() / kfree().
> > > + trace_rcu_invoke_kfree_bulk_callback(
> > > + rcu_state.name, bkvhead[i]->nr_records,
> > > + bkvhead[i]->records);
> > > +
> > > + kfree_bulk(bkvhead[i]->nr_records,
> > > + bkvhead[i]->records);
> > > + } else { // vmalloc() / vfree().
> > > + for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > > + trace_rcu_invoke_kfree_callback(
> > > + rcu_state.name,
> > > + bkvhead[i]->records[j], 0);
> > > +
> > > + vfree(bkvhead[i]->records[j]);
> > > + }
> > > + }
> > > + rcu_lock_release(&rcu_callback_map);
> > 
> > Not an emergency, but did you look into replacing this "if" statement
> > with an array of pointers to functions implementing the legs of the
> > "if" statement?  If nothing else, this would greatly reduced indentation.
> 
> I don't think that replacing direct function calls with indirect function
> calls is a great suggestion with the current state of play around branch
> prediction.
> 
> I'd suggest:
> 
>   rcu_lock_acquire(&rcu_callback_map);
>   trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
>   bkvhead[i]->nr_records, bkvhead[i]->records);
>   if (i == 0) {
>   kfree_bulk(bkvhead[i]->nr_records,
>   bkvhead[i]->records);
>   } else {
>   for (j = 0; j < bkvhead[i]->nr_records; j++) {
>   vfree(bkvhead[i]->records[j]);
>   }
>   }
>   rcu_lock_release(&rcu_callback_map);
> 
> But I'd also suggest a vfree_bulk be added.  There are a few things
> which would be better done in bulk as part of the vfree process
> (we batch them up already, but i'm sure we could do better).

I suspect that he would like to keep the tracing.

It might be worth trying the branches, given that they would be constant
and indexed by "i".  The compiler might well remove the indirection.

The compiler guys brag about doing so, which of course might or might
not have any correlation to a given compiler actually doing so.  :-/

Having a vfree_bulk() might well be useful, but I would feel more
confidence in that if there were other callers of kfree_bulk().

But again, either way, future work as far as this series is concerned.

Thanx, Paul

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Matthew Wilcox]

On Wed, Jun 17, 2020 at 04:46:09PM -0700, Paul E. McKenney wrote:
> > +   // Handle two first channels.
> > +   for (i = 0; i < FREE_N_CHANNELS; i++) {
> > +   for (; bkvhead[i]; bkvhead[i] = bnext) {
> > +   bnext = bkvhead[i]->next;
> > +   debug_rcu_bhead_unqueue(bkvhead[i]);
> > +
> > +   rcu_lock_acquire(&rcu_callback_map);
> > +   if (i == 0) { // kmalloc() / kfree().
> > +   trace_rcu_invoke_kfree_bulk_callback(
> > +   rcu_state.name, bkvhead[i]->nr_records,
> > +   bkvhead[i]->records);
> > +
> > +   kfree_bulk(bkvhead[i]->nr_records,
> > +   bkvhead[i]->records);
> > +   } else { // vmalloc() / vfree().
> > +   for (j = 0; j < bkvhead[i]->nr_records; j++) {
> > +   trace_rcu_invoke_kfree_callback(
> > +   rcu_state.name,
> > +   bkvhead[i]->records[j], 0);
> > +
> > +   vfree(bkvhead[i]->records[j]);
> > +   }
> > +   }
> > +   rcu_lock_release(&rcu_callback_map);
> 
> Not an emergency, but did you look into replacing this "if" statement
> with an array of pointers to functions implementing the legs of the
> "if" statement?  If nothing else, this would greatly reduced indentation.

I don't think that replacing direct function calls with indirect function
calls is a great suggestion with the current state of play around branch
prediction.

I'd suggest:

rcu_lock_acquire(&rcu_callback_map);
trace_rcu_invoke_kfree_bulk_callback(rcu_state.name,
bkvhead[i]->nr_records, bkvhead[i]->records);
if (i == 0) {
kfree_bulk(bkvhead[i]->nr_records,
bkvhead[i]->records);
} else {
for (j = 0; j < bkvhead[i]->nr_records; j++) {
vfree(bkvhead[i]->records[j]);
}
}
rcu_lock_release(&rcu_callback_map);

But I'd also suggest a vfree_bulk be added.  There are a few things
which would be better done in bulk as part of the vfree process
(we batch them up already, but i'm sure we could do better).

Re: [PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Paul E. McKenney]

On Mon, May 25, 2020 at 11:47:53PM +0200, Uladzislau Rezki (Sony) wrote:
> To do so, we use an array of kvfree_rcu_bulk_data structures.
> It consists of two elements:
>  - index number 0 corresponds to slab pointers.
>  - index number 1 corresponds to vmalloc pointers.
> 
> Keeping vmalloc pointers separated from slab pointers makes
> it possible to invoke the right freeing API for the right
> kind of pointer.
> 
> It also prepares us for future headless support for vmalloc
> and SLAB objects. Such objects cannot be queued on a linked
> list and are instead directly into an array.
> 
> Signed-off-by: Uladzislau Rezki (Sony) 
> Signed-off-by: Joel Fernandes (Google) 
> Reviewed-by: Joel Fernandes (Google) 
> Co-developed-by: Joel Fernandes (Google) 
> ---

[ . . . ]

> + // Handle two first channels.
> + for (i = 0; i < FREE_N_CHANNELS; i++) {
> + for (; bkvhead[i]; bkvhead[i] = bnext) {
> + bnext = bkvhead[i]->next;
> + debug_rcu_bhead_unqueue(bkvhead[i]);
> +
> + rcu_lock_acquire(&rcu_callback_map);
> + if (i == 0) { // kmalloc() / kfree().
> + trace_rcu_invoke_kfree_bulk_callback(
> + rcu_state.name, bkvhead[i]->nr_records,
> + bkvhead[i]->records);
> +
> + kfree_bulk(bkvhead[i]->nr_records,
> + bkvhead[i]->records);
> + } else { // vmalloc() / vfree().
> + for (j = 0; j < bkvhead[i]->nr_records; j++) {
> + trace_rcu_invoke_kfree_callback(
> + rcu_state.name,
> + bkvhead[i]->records[j], 0);
> +
> + vfree(bkvhead[i]->records[j]);
> + }
> + }
> + rcu_lock_release(&rcu_callback_map);

Not an emergency, but did you look into replacing this "if" statement
with an array of pointers to functions implementing the legs of the
"if" statement?  If nothing else, this would greatly reduced indentation.

I am taking this as is, but if you have not already done so, could you
please look into this for a follow-up patch?

Thanx, Paul

[PATCH v2 09/16] rcu/tree: Maintain separate array for vmalloc ptrs

[Uladzislau Rezki (Sony)]

To do so, we use an array of kvfree_rcu_bulk_data structures.
It consists of two elements:
 - index number 0 corresponds to slab pointers.
 - index number 1 corresponds to vmalloc pointers.

Keeping vmalloc pointers separated from slab pointers makes
it possible to invoke the right freeing API for the right
kind of pointer.

It also prepares us for future headless support for vmalloc
and SLAB objects. Such objects cannot be queued on a linked
list and are instead directly into an array.

Signed-off-by: Uladzislau Rezki (Sony) 
Signed-off-by: Joel Fernandes (Google) 
Reviewed-by: Joel Fernandes (Google) 
Co-developed-by: Joel Fernandes (Google) 
---
 kernel/rcu/tree.c | 173 +++---
 1 file changed, 100 insertions(+), 73 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index e2267e92de5d..9f84ff80bc25 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -57,6 +57,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "../time/tick-internal.h"
 
 #include "tree.h"
@@ -2832,46 +2834,47 @@ EXPORT_SYMBOL_GPL(call_rcu);
 /* Maximum number of jiffies to wait before draining a batch. */
 #define KFREE_DRAIN_JIFFIES (HZ / 50)
 #define KFREE_N_BATCHES 2
+#define FREE_N_CHANNELS 2
 
 /**
- * struct kfree_rcu_bulk_data - single block to store kfree_rcu() pointers
+ * struct kvfree_rcu_bulk_data - single block to store kvfree_rcu() pointers
  * @nr_records: Number of active pointers in the array
- * @records: Array of the kfree_rcu() pointers
  * @next: Next bulk object in the block chain
+ * @records: Array of the kvfree_rcu() pointers
  */
-struct kfree_rcu_bulk_data {
+struct kvfree_rcu_bulk_data {
unsigned long nr_records;
-   struct kfree_rcu_bulk_data *next;
+   struct kvfree_rcu_bulk_data *next;
void *records[];
 };
 
 /*
  * This macro defines how many entries the "records" array
  * will contain. It is based on the fact that the size of
- * kfree_rcu_bulk_data structure becomes exactly one page.
+ * kvfree_rcu_bulk_data structure becomes exactly one page.
  */
-#define KFREE_BULK_MAX_ENTR \
-   ((PAGE_SIZE - sizeof(struct kfree_rcu_bulk_data)) / sizeof(void *))
+#define KVFREE_BULK_MAX_ENTR \
+   ((PAGE_SIZE - sizeof(struct kvfree_rcu_bulk_data)) / sizeof(void *))
 
 /**
  * struct kfree_rcu_cpu_work - single batch of kfree_rcu() requests
  * @rcu_work: Let queue_rcu_work() invoke workqueue handler after grace period
  * @head_free: List of kfree_rcu() objects waiting for a grace period
- * @bhead_free: Bulk-List of kfree_rcu() objects waiting for a grace period
+ * @bkvhead_free: Bulk-List of kvfree_rcu() objects waiting for a grace period
  * @krcp: Pointer to @kfree_rcu_cpu structure
  */
 
 struct kfree_rcu_cpu_work {
struct rcu_work rcu_work;
struct rcu_head *head_free;
-   struct kfree_rcu_bulk_data *bhead_free;
+   struct kvfree_rcu_bulk_data *bkvhead_free[FREE_N_CHANNELS];
struct kfree_rcu_cpu *krcp;
 };
 
 /**
  * struct kfree_rcu_cpu - batch up kfree_rcu() requests for RCU grace period
  * @head: List of kfree_rcu() objects not yet waiting for a grace period
- * @bhead: Bulk-List of kfree_rcu() objects not yet waiting for a grace period
+ * @bkvhead: Bulk-List of kvfree_rcu() objects not yet waiting for a grace 
period
  * @krw_arr: Array of batches of kfree_rcu() objects waiting for a grace period
  * @lock: Synchronize access to this structure
  * @monitor_work: Promote @head to @head_free after KFREE_DRAIN_JIFFIES
@@ -2886,7 +2889,7 @@ struct kfree_rcu_cpu_work {
  */
 struct kfree_rcu_cpu {
struct rcu_head *head;
-   struct kfree_rcu_bulk_data *bhead;
+   struct kvfree_rcu_bulk_data *bkvhead[FREE_N_CHANNELS];
struct kfree_rcu_cpu_work krw_arr[KFREE_N_BATCHES];
raw_spinlock_t lock;
struct delayed_work monitor_work;
@@ -2910,7 +2913,7 @@ static DEFINE_PER_CPU(struct kfree_rcu_cpu, krc) = {
 };
 
 static __always_inline void
-debug_rcu_bhead_unqueue(struct kfree_rcu_bulk_data *bhead)
+debug_rcu_bhead_unqueue(struct kvfree_rcu_bulk_data *bhead)
 {
 #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
int i;
@@ -2939,20 +2942,20 @@ krc_this_cpu_unlock(struct kfree_rcu_cpu *krcp, 
unsigned long flags)
local_irq_restore(flags);
 }
 
-static inline struct kfree_rcu_bulk_data *
+static inline struct kvfree_rcu_bulk_data *
 get_cached_bnode(struct kfree_rcu_cpu *krcp)
 {
if (!krcp->nr_bkv_objs)
return NULL;
 
krcp->nr_bkv_objs--;
-   return (struct kfree_rcu_bulk_data *)
+   return (struct kvfree_rcu_bulk_data *)
llist_del_first(&krcp->bkvcache);
 }
 
 static inline bool
 put_cached_bnode(struct kfree_rcu_cpu *krcp,
-   struct kfree_rcu_bulk_data *bnode)
+   struct kvfree_rcu_bulk_data *bnode)
 {
// Check the limit.
if (krcp->nr_bkv_objs >= rcu_min_cached_objs)
@@ -2971,43 +2974,63 @@ put_cached_bnode(struct kfree_rcu_cpu *krcp,
 static void kfree_rcu_w