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

Paul E. McKenney
2011-03-17T17:50:51Z

On Thu, Mar 17, 2011 at 05:28:49PM +0800, Lai Jiangshan wrote:
> On 03/16/2011 12:03 PM, Paul E. McKenney wrote:
> > On Tue, Mar 15, 2011 at 02:11:33PM +0100, Eric Dumazet wrote:
> >> Le mardi 15 mars 2011 à 17:46 +0800, Lai Jiangshan a écrit :
> >>
> >>
> >>> --- 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++);
> >>
> >> Paul, I am just wondering why we disable BH before calling list->func()
> >>
> >> This should be done in callbacks that really need it ?
> >>
> >> At least the disable/enable pair is not necessary before calling kfree()
> >
> > Good point, we could bury the enable/disable pair in __rcu_reclaim().
> >
> > Lai, am I forgetting any reason why we disable BH?
> >
> > Thanx, Paul
> >
>
> For many years, rcu callbacks are called on BH since rcu is added to kernel,
> and someone assume they always called in BH. So we have to disable BH before
> calling list->func() to avoid bad result. It's a *historical* reason.
>
> I greed the disable/enable pair is not necessary before calling kfree(), but
> __rcu_reclaim() is also called in rcutree which rcu_process_callbacks()
> is in BH currently, I don't want to write 2 different version of
> __rcu_reclaim()s (one for rcutree, another for rcutiny).
>
> rcutree's rcu_process_callbacks() will be moved to process context, we may
> remove disable/enable BH pair for kfree() then.

OK, so if I sequence your patches after the rcutree priority boosting,
which threadifies rcutree's callback processing, I should be able to
omit BH for the kfree() case.

Thanx, Paul