Hi Paul,
Thank you for your comments, and sorry for my late reply.
On 2012/09/21 2:34, Paul E. McKenney wrote:
> On Thu, Sep 06, 2012 at 08:27:40PM +0900, Tomoki Sekiyama wrote:
>> Initialize rcu related variables to avoid warnings about RCU usage while
>> slave CPUs is running specified functions. Also notify RCU subsystem before
>> the slave CPU is entered into idle state.
>
> Hello, Tomoki,
>
> A few questions and comments interspersed below.
>> <snip>
>> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
>> index e8cfe377..45dfc1d 100644
>> --- a/arch/x86/kernel/smpboot.c
>> +++ b/arch/x86/kernel/smpboot.c
>> @@ -382,6 +382,8 @@ notrace static void __cpuinit start_slave_cpu(void *unused)
>> f = per_cpu(slave_cpu_func, cpu);
>> per_cpu(slave_cpu_func, cpu).func = NULL;
>>
>> + rcu_note_context_switch(cpu);
>> +
>
> Why not use rcu_idle_enter() and rcu_idle_exit()? These would tell
> RCU to ignore the slave CPU for the duration of its idle period.
> The way you have it, if a slave CPU stayed idle for too long, you
> would get RCU CPU stall warnings, and possibly system hangs as well.
That's true, rcu_idle_enter() and rcu_idle_exit() should be used when
the slave cpu is idle. Thanks.
> Or is this being called from some task that is not the idle task?
> If so, you instead want the new rcu_user_enter() and rcu_user_exit()
> that are hopefully on their way into 3.7. Or maybe better, use a real
> idle task, so that idle_task(smp_processor_id()) returns true and RCU
> stops complaining. ;-)
>
> Note that CPUs that RCU believes to be idle are not permitted to contain
> RCU read-side critical sections, which in turn means no entering the
> scheduler, no sleeping, and so on. There is an RCU_NONIDLE() macro
> to tell RCU to pay attention to the CPU only for the duration of the
> statement passed to RCU_NONIDLE, and there are also an _rcuidle variant
> of the tracing statement to allow tracing from idle.
This was for KVM is called as `func', which contains RCU read-side critical
sections, and rcu_virt_note_context_switch() (that is
rcu_note_context_switch(cpu)) before entering guest.
Maybe it should be replaced by rcu_user_enter() and rcu_user_exit() in the
future.
>> --- a/kernel/rcutree.c
>> +++ b/kernel/rcutree.c
>> @@ -2589,6 +2589,9 @@ static int __cpuinit rcu_cpu_notify(struc tnotifier_block *self,
>> switch (action) {
>> case CPU_UP_PREPARE:
>> case CPU_UP_PREPARE_FROZEN:
>> +#ifdef CONFIG_SLAVE_CPU
>> + case CPU_SLAVE_UP_PREPARE:
>> +#endif
>
> Why do you need #ifdef here? Why not define CPU_SLAVE_UP_PREPARE
> unconditionally? Then if CONFIG_SLAVE_CPU=n, rcu_cpu_notify() would
> never be invoked with CPU_SLAVE_UP_PREPARE, so no problems.
Agreed. That will make the code simpler.
Thank you again,
--
Tomoki Sekiyama <tomoki.sekiyama.qu@xxxxxxxxxxx>
Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
