Re: [RFC PATCH 1/3] cpu-hotplug: Refcount Based Cpu Hotplug implementation

[Oleg Nesterov <oleg@tv-sign.ru>]

On 11/15, Gautham R Shenoy wrote:
>
> +static struct {
> +	struct task_struct *active_writer;
> +	struct mutex lock; /* Synchronizes accesses to refcount, */
> +	/*
> +	 * Also blocks the new readers during
> +	 * an ongoing cpu hotplug operation.
> +	 */
> +	int refcount;
> +	wait_queue_head_t writer_queue;
> +} cpu_hotplug;
> ...
>  void unlock_cpu_hotplug(void)
>  {
> -	WARN_ON(recursive != current);
> -	if (recursive_depth) {
> -		recursive_depth--;
> +	if (cpu_hotplug.active_writer == current)
>  		return;
> -	}
> -	recursive = NULL;
> -	mutex_unlock(&cpu_bitmask_lock);
> +	mutex_lock(&cpu_hotplug.lock);
> +	cpu_hotplug.refcount--;
> +
> +	if (unlikely(writer_exists()) && !cpu_hotplug.refcount)
> +		wake_up(&cpu_hotplug.writer_queue);
> +
> +	mutex_unlock(&cpu_hotplug.lock);
> +
>  }
> ...
> +static void cpu_hotplug_begin(void)
> +{
> +	DECLARE_WAITQUEUE(wait, current);
> +
> +	mutex_lock(&cpu_hotplug.lock);
> +
> +	cpu_hotplug.active_writer = current;
> +	add_wait_queue_exclusive(&cpu_hotplug.writer_queue, &wait);
> +	while (cpu_hotplug.refcount) {
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		mutex_unlock(&cpu_hotplug.lock);
> +		schedule();
> +		mutex_lock(&cpu_hotplug.lock);
> +	}
> +	remove_wait_queue_locked(&cpu_hotplug.writer_queue, &wait);
> +}

Perhaps we can simplify this a little bit? I don't think we really need
cpu_hotplug.writer_queue, afaics we can just do

	void unlock_cpu_hotplug(void)
	{
		if (cpu_hotplug.active_writer == current)
			return;
		mutex_lock(&cpu_hotplug.lock);
		if (!--cpu_hotplug.refcount && cpu_hotplug.active_writer)
			wake_up_process(cpu_hotplug.active_writer);
		mutex_unlock(&cpu_hotplug.lock);

	}

	static void cpu_hotplug_begin(void)
	{
		mutex_lock(&cpu_hotplug.lock);
		cpu_hotplug.active_writer = current;

		while (cpu_hotplug.refcount) {
			__set_current_state(TASK_UNINTERRUPTIBLE);
			mutex_unlock(&cpu_hotplug.lock);
			schedule();
			mutex_lock(&cpu_hotplug.lock);
		}
	}

(not that it matters, we can do this later even if I am right)

Oleg.