Re: [PATCH] Introduce down_nowait()

From: Andrew Morton
Date: Wed May 21 2008 - 02:29:31 EST


On Wed, 21 May 2008 16:00:15 +1000 Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote:

> I planned on removing the much-disliked down_trylock() (with its
> backwards return codes) in 2.6.27, but it's creating something of a
> logjam with other patches in -mm and linux-next.
>
> Andrew suggested introducing "down_nowait" as a wrapper now, to make
> the transition easier.
>
> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Cc: Matthew Wilcox <matthew@xxxxxx>
> Cc: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
>
> diff -r 92664ae4130b include/linux/semaphore.h
> --- a/include/linux/semaphore.h Wed May 21 14:54:40 2008 +1000
> +++ b/include/linux/semaphore.h Wed May 21 15:07:31 2008 +1000
> @@ -48,4 +48,18 @@ extern int __must_check down_timeout(str
> extern int __must_check down_timeout(struct semaphore *sem, long jiffies);
> extern void up(struct semaphore *sem);
>
> +/**
> + * down_nowait - try to down a semaphore, but don't block
> + * @sem: the semaphore
> + *
> + * This is equivalent to down_trylock(), but has the same return codes as
> + * spin_trylock and mutex_trylock: 1 if semaphore acquired, 0 if not.
> + *
> + * down_trylock() with its confusing return codes will be deprecated
> + * soon. It will not be missed.
> + */
> +static inline int __must_check down_nowait(struct semaphore *sem)
> +{
> + return !down_trylock(sem);
> +}
> #endif /* __LINUX_SEMAPHORE_H */

Actually, I don't thing down_nowait() is a terribly good name, because it
doesn't tell the reader anything about what to expect from the return
value. Does a non-zero return mean that down_wait() acquired the lock,
or does it not? Something like down_try() would be better, because if
it returns 1 we can say "ah, the trying succeeded".

otoh if all the kernel's "try" functions now return true on successful
acquisition then there won't be any confusion any more so the name
problably doesn't matter.

Except "down_nowait" doesn't have "try" in its name. down_try() would
be better?

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