linux-kernel - Re: [PATCH] simplify cpu_hotplug_begin()/put_online

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080218145947.GA27127@in.ibm.com>
Date:	Mon, 18 Feb 2008 20:29:47 +0530
From:	Gautham R Shenoy <ego@...ibm.com>
To:	Oleg Nesterov <oleg@...sign.ru>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Dipankar Sarma <dipankar@...ibm.com>,
	Ingo Molnar <mingo@...e.hu>,
	Srivatsa Vaddagiri <vatsa@...ibm.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] simplify cpu_hotplug_begin()/put_online_cpus()

On Sat, Feb 16, 2008 at 08:22:54PM +0300, Oleg Nesterov wrote:

This looks neat and clean.

Acked-by: Gautham R Shenoy <ego@...ibm.com>


> cpu_hotplug_begin() must be always called under cpu_add_remove_lock, this means
> that only one process can be cpu_hotplug.active_writer. So we don't need the
> cpu_hotplug.writer_queue, we can wake up the ->active_writer directly.
> 
> Also, fix the comment.
> 
> Signed-off-by: Oleg Nesterov <oleg@...sign.ru>
> 
> --- 25/kernel/cpu.c~1_CPU_HP_LOCK	2008-02-15 16:59:17.000000000 +0300
> +++ 25/kernel/cpu.c	2008-02-16 18:36:37.000000000 +0300
> @@ -33,17 +33,13 @@ static struct {
>  	 * an ongoing cpu hotplug operation.
>  	 */
>  	int refcount;
> -	wait_queue_head_t writer_queue;
>  } cpu_hotplug;
> 
> -#define writer_exists() (cpu_hotplug.active_writer != NULL)
> -
>  void __init cpu_hotplug_init(void)
>  {
>  	cpu_hotplug.active_writer = NULL;
>  	mutex_init(&cpu_hotplug.lock);
>  	cpu_hotplug.refcount = 0;
> -	init_waitqueue_head(&cpu_hotplug.writer_queue);
>  }
> 
>  #ifdef CONFIG_HOTPLUG_CPU
> @@ -65,11 +61,8 @@ void put_online_cpus(void)
>  	if (cpu_hotplug.active_writer == current)
>  		return;
>  	mutex_lock(&cpu_hotplug.lock);
> -	cpu_hotplug.refcount--;
> -
> -	if (unlikely(writer_exists()) && !cpu_hotplug.refcount)
> -		wake_up(&cpu_hotplug.writer_queue);
> -
> +	if (!--cpu_hotplug.refcount && unlikely(cpu_hotplug.active_writer))
> +		wake_up_process(cpu_hotplug.active_writer);
>  	mutex_unlock(&cpu_hotplug.lock);
> 
>  }
> @@ -98,8 +91,8 @@ void cpu_maps_update_done(void)
>   * Note that during a cpu-hotplug operation, the new readers, if any,
>   * will be blocked by the cpu_hotplug.lock
>   *
> - * Since cpu_maps_update_begin is always called after invoking
> - * cpu_maps_update_begin, we can be sure that only one writer is active.
> + * Since cpu_hotplug_begin() is always called after invoking
> + * cpu_maps_update_begin(), we can be sure that only one writer is active.
>   *
>   * Note that theoretically, there is a possibility of a livelock:
>   * - Refcount goes to zero, last reader wakes up the sleeping
> @@ -115,19 +108,16 @@ void cpu_maps_update_done(void)
>   */
>  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);
> +
> +	for (;;) {
> +		mutex_lock(&cpu_hotplug.lock);
> +		if (likely(!cpu_hotplug.refcount))
> +			break;
> +		__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);
>  }
> 
>  static void cpu_hotplug_done(void)

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