Re: [PATCH 04/11] rwsem: lighter active count checks when waking up readers

[Daniel Walker <dwalker@fifo99.com>]

On Mon, 2010-05-24 at 13:31 -0700, Michel Lespinasse wrote:
> In __rwsem_do_wake(), we can skip the active count check unless we come
> there from up_xxxx(). Also when checking the active count, it is not
> actually necessary to increment it; this allows us to get rid of the
> read side undo code and simplify the calculation of the final rwsem count
> adjustment once we've counted the reader threads to wake.
> 
> The basic observation is the following. When there are waiter threads
> on a rwsem and the spinlock is held, other threads can only increment the
> active count by trying to grab the rwsem in down_xxxx(). However down_xxxx()
> will notice there are waiter threads and take the down_failed path,
> blocking to acquire the spinlock on the way there. Therefore, a thread
> observing an active count of zero with waiters queued and the spinlock held,
> is protected against other threads acquiring the rwsem until it wakes the
> last waiter or releases the spinlock.
> 
> Signed-off-by: Michel Lespinasse <walken@google.com>
> ---
>  lib/rwsem.c |   57 ++++++++++++++++++++++++++++++++-------------------------
>  1 files changed, 32 insertions(+), 25 deletions(-)
> 
> diff --git a/lib/rwsem.c b/lib/rwsem.c
> index 917fd94..94f2d7a 100644
> --- a/lib/rwsem.c
> +++ b/lib/rwsem.c
> @@ -36,6 +36,14 @@ struct rwsem_waiter {
>  #define RWSEM_WAITING_FOR_WRITE	0x00000002
>  };
>  
> +/* Wake types for __rwsem_do_wake().  Note that RWSEM_WAKE_NO_ACTIVE and
> + * RWSEM_WAKE_READ_OWNED imply that the spinlock must have been kept held
> + * since the rwsem value was observed.
> + */
> +#define RWSEM_WAKE_ANY        0 /* Wake whatever's at head of wait list */
> +#define RWSEM_WAKE_NO_ACTIVE  1 /* rwsem was observed with no active thread */
> +#define RWSEM_WAKE_READ_OWNED 2 /* rwsem was observed to be read owned */
> +
>  /*
>   * handle the lock release when processes blocked on it that can now run
>   * - if we come here from up_xxxx(), then:
> @@ -46,8 +54,8 @@ struct rwsem_waiter {
>   * - woken process blocks are discarded from the list after having task zeroed
>   * - writers are only woken if downgrading is false
>   */
> -static inline struct rw_semaphore *
> -__rwsem_do_wake(struct rw_semaphore *sem, int downgrading)
> +static struct rw_semaphore *
> +__rwsem_do_wake(struct rw_semaphore *sem, int wake_type)

You could convert the "wake_type" into an enum along with the defines
above.

Daniel