linux-kernel - Re: [RFC] [PATCH] Lightweight kernel condition variables: faster code, fewer bugs

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]

Date:	Thu, 28 Jul 2011 13:00:59 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Chris <chris@...t.co.uk>
cc:	Linux Kernel <linux-kernel@...r.kernel.org>, <mingo@...hat.com>
Subject: Re: [RFC] [PATCH] Lightweight kernel condition variables: faster
 code, fewer bugs

On Wed, 27 Jul 2011, Chris Simmonds wrote:

> Hi,
> 
> This patch adds lightweight condition variables to the kernel to reduce 
> complexity and improve the efficiency of some synchronisation tasks. 
> They are very similar to POSIX condition variables.

...

> To wake up a thread sleeping on a condition variable you just use the 
> normal wakeup calls, nothing new there. Except of course that you need 
> to consider the locking round the variables (condition) that was just 
> modified, but that should be in place already. There are many other 
> examples in the code which could be improved in this way.
> 
> The patch that follows implements condition variables using mutexes and 
> various sorts of spinlock as the locking primitive. I am aware that I 
> have not covered all possibilities and that the code could be neater. At 
> this point I just want to show the idea and get feedback.

It seems like a reasonable sort of thing to do, as far as I can see.

> diff --git a/include/linux/wait.h b/include/linux/wait.h
> index 3efc9f3..76f9c25 100644
> --- a/include/linux/wait.h
> +++ b/include/linux/wait.h
> @@ -662,7 +662,89 @@ static inline int wait_on_bit_lock(void *word, int bit,
>   		return 0;
>   	return out_of_line_wait_on_bit_lock(word, bit, action, mode);
>   }
> -	
> +
> +/**
> + * cond_wait - wait for a condition variable
> + * @wq: the wait queue to wait on
> + * @mutex: a locked mutex
> + *
> + * Safely put the calling task into a non-interruptible sleep on the
> + * wait queue then unlock the mutex. Re-acquire the mutex after waking up
> + */
> +void cond_wait (wait_queue_head_t *wq, struct mutex *mutex)
> +{
> +	DEFINE_WAIT(__wait);
> +
> +	prepare_to_wait(wq, &__wait, TASK_UNINTERRUPTIBLE);
> +	mutex_unlock (mutex);
> +	schedule();
> +	mutex_lock (mutex);
> +	finish_wait(wq, &__wait);
> +}

One little problem here.  These routines may well be large enough
that it's inefficient to inline them.  In that case they should be 
declared in wait.h but defined somewhere else, such as kernel/wait.c.  
Conversely, if you do think they deserve to be inlined then they should 
be marked as such.

Alan Stern

P.S.: Does this come through checkpatch.pl unscathed?

--
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/