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

[Lai Jiangshan]

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.

Thanks,
Lai.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/