Re: [PATCH V4 1/1] rcu: introduce kfree_rcu()

Paul E. McKenney
2011-03-15T11:30:46Z

On Tue, Mar 15, 2011 at 05:46:20PM +0800, Lai Jiangshan wrote:
> kfree_rcu() which was original proposed by Lai 2.5 years ago is one of
> the most important RCU TODO list entries, Lai and Manfred have worked on
> patches for this. This V4 patch is based on the Manfred's patch and
> the V1 of Lai's patch. (These two patches are almost the same
> in implementation, and this patch is mainly based on the Manfred's).
>
> Lai's V1 patch: http://lkml.org/lkml/2008/9/18/1
> Manfred's patch: http://lkml.org/lkml/2009/1/2/115
> RCU TODO list: http://www.kernel.org/pub/linux/kernel/people/paulmck/rcutodo.html
>
> This new introduced API kfree_rcu() primitive kfree()s the specified memory
> after a RCU grace period elapses.
>
> It replaces many simple "call_rcu(head, simple_kfree_callback)";
> These many simple_kfree_callback() instances just does
>
> kfree(containerof(head,struct whatever_struct,rcu_member));
>
> These simple_kfree_callback() instances are just duplicate code, we need
> a generic function for them.
>
> And kfree_rcu() is also help for unloadable modules, kfree_rcu() does not
> queue any function which belong to the module, so a rcu_barrier() can
> be avoid when module exit. (If we queue any other function by call_rcu(),
> rcu_barrier() is still needed.)

Thank you for putting this together! It does represent a nice
reduction in code size.

Once it settles out a bit, I intend to queue this patch. It would be
best if the subsystems queue their own patches using kfree_rcu() once
this patch reaches mainline.

Seem reasonable?

Thanx, Paul

> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
> ---
> include/linux/rcupdate.h | 40 ++++++++++++++++++++++++++++++++++++++++
> kernel/rcutiny.c | 2 +-
> kernel/rcutree.c | 2 +-
> 3 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 7d62909..18f7ade 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -777,4 +777,44 @@ static inline void debug_rcu_head_unqueue(struct rcu_head *head)
> }
> #endif /* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
>
> +static __always_inline bool __is_kfree_rcu_offset(unsigned long offset)
> +{
> + return offset < 4096;
> +}
> +
> +static __always_inline
> +void __kfree_rcu(struct rcu_head *head, unsigned long offset)
> +{
> + typedef void (*rcu_callback)(struct rcu_head *);
> +
> + BUILD_BUG_ON(!__builtin_constant_p(offset));
> + BUILD_BUG_ON(!__is_kfree_rcu_offset(offset));
> +
> + call_rcu(head, (rcu_callback)offset);
> +}
> +
> +extern void kfree(const void *);
> +
> +static inline void __rcu_reclaim(struct rcu_head *head)
> +{
> + unsigned long offset = (unsigned long)head->func;
> +
> + if (__is_kfree_rcu_offset(offset))
> + kfree((void *)head - offset);
> + else
> + head->func(head);
> +}
> +
> +/**
> + * kfree_rcu() - kfree an object after a grace period.
> + * @ptr: pointer to kfree
> + * @rcu_head: the name of the struct rcu_head within the type of @ptr.
> + *
> + * Many rcu callbacks just call kfree() on the base structure. This helper
> + * function calls kfree internally. The rcu_head structure must be embedded
> + * in the to be freed structure.
> + */
> +#define kfree_rcu(ptr, rcu_head) \
> + __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
> +
> #endif /* __LINUX_RCUPDATE_H */
> diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
> index 0c343b9..4d60fbc 100644
> --- a/kernel/rcutiny.c
> +++ b/kernel/rcutiny.c
> @@ -167,7 +167,7 @@ static void rcu_process_callbacks(struct rcu_ctrlblk *rcp)
> prefetch(next);
> debug_rcu_head_unqueue(list);
> local_bh_disable();
> - list->func(list);
> + __rcu_reclaim(list);
> local_bh_enable();
> list = next;
> RCU_TRACE(cb_count++);
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index dd4aea8..b3c1aed 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -1143,7 +1143,7 @@ static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp)
> next = list->next;
> prefetch(next);
> debug_rcu_head_unqueue(list);
> - list->func(list);
> + __rcu_reclaim(list);
> list = next;
> if (++count >= rdp->blimit)
> break;
> --
> 1.7.4