I'll try to read this series later,
one minor and almost offtopic nit.
On 12/06, Srivatsa S. Bhat wrote:
>
> static int __ref take_cpu_down(void *_param)
> {
> struct take_cpu_down_param *param = _param;
> + unsigned long flags;
> int err;
>
> + /*
> + * __cpu_disable() is the step where the CPU is removed from the
> + * cpu_online_mask. Protect it with the light-lock held for write.
> + */
> + write_lock_irqsave(&light_hotplug_rwlock, flags);
> +
> /* Ensure this CPU doesn't handle any more interrupts. */
> err = __cpu_disable();
> - if (err < 0)
> + if (err < 0) {
> + write_unlock_irqrestore(&light_hotplug_rwlock, flags);
> return err;
> + }
> +
> + /*
> + * We have successfully removed the CPU from the cpu_online_mask.
> + * So release the light-lock, so that the light-weight atomic readers
> + * (who care only about the cpu_online_mask updates, and not really
> + * about the actual cpu-take-down operation) can continue.
> + *
> + * But don't enable interrupts yet, because we still have work left to
> + * do, to actually bring the CPU down.
> + */
> + write_unlock(&light_hotplug_rwlock);
>
> cpu_notify(CPU_DYING | param->mod, param->hcpu);
> +
> + local_irq_restore(flags);
> return 0;
This is subjective, but imho _irqsave and the fat comment look confusing.
Currently take_cpu_down() is always called with irqs disabled, so you
do not need to play with interrupts.
10/10 does s/__stop_machine/stop_cpus/ and that patch could simply add
local_irq_disable/enable into take_cpu_down().
But again this is minor and subjective, I won't insist.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
