Re: [PATCH 2.6.24-mm1 1/8] (resend) IPC/semaphores: code factorisation

[Alexey Dobriyan <adobriyan@gmail.com>]

On Tue, Feb 12, 2008 at 05:13:41PM +0100, pierre.peiffer@bull.net wrote:
> Trivial patch which adds some small locking functions and makes use of them
> to factorize some part of the code and to make it cleaner.

What's wrong with consolidation activity in general is that one need to
follow tags many times to realise what on earth function really does.

> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -180,6 +180,25 @@ static inline struct sem_array *sem_lock
>  	return container_of(ipcp, struct sem_array, sem_perm);
>  }
>  
> +static inline void sem_lock_and_putref(struct sem_array *sma)
> +{
> +	ipc_lock_by_ptr(&sma->sem_perm);
> +	ipc_rcu_putref(sma);
> +}
> +
> +static inline void sem_getref_and_unlock(struct sem_array *sma)
> +{
> +	ipc_rcu_getref(sma);
> +	ipc_unlock(&(sma)->sem_perm);
> +}
> +
> +static inline void sem_putref(struct sem_array *sma)
> +{

It should be called sem_lock_putref_and_unlock() at which point it
becomes obvious that this patch is from useless-wrappers land.

> +	ipc_lock_by_ptr(&sma->sem_perm);
> +	ipc_rcu_putref(sma);
> +	ipc_unlock(&(sma)->sem_perm);

On the other hand PUT is associated with atomic_dec_and_test() which
does no locking itself.