Re: [patch 5/9] mutex subsystem, core

[Christoph Hellwig <hch@infradead.org>]

> +#include <linux/config.h>

we don't need config.h anymore, it's included implicitly now.

> +#include <asm/atomic.h>

Any chance we could include this after the <linux/*.h> headers ?

> +#include <linux/spinlock_types.h>

What do we need this one for?

> +struct mutex {
> +	// 1: unlocked, 0: locked, negative: locked, possible waiters

please use /* */ comments.

> +	atomic_t		count;
> +	spinlock_t		wait_lock;
> +	struct list_head	wait_list;
> +#ifdef CONFIG_DEBUG_MUTEXES
> +	struct thread_info	*owner;
> +	struct list_head	held_list;
> +	unsigned long		acquire_ip;
> +	const char 		*name;
> +	void			*magic;
> +#endif
> +};

I know we generally don't like typedefs, but mutex is like spinlocks one
of those cases where the internals should be completely opaqueue, so a mutex_t
sounds like a good idea.

> +#include <linux/syscalls.h>

What do you we need this header for?

> +static inline void __mutex_lock_atomic(struct mutex *lock)
> +{
> +#ifdef __ARCH_WANT_XCHG_BASED_ATOMICS
> +	if (unlikely(atomic_xchg(&lock->count, 0) != 1))
> +		__mutex_lock_noinline(&lock->count);
> +#else
> +	atomic_dec_call_if_negative(&lock->count, __mutex_lock_noinline);
> +#endif
> +}

this is the kind of thing I meant in the comment to the announcement.

Just having this in arch code would kill all these ifdefs  over mutex.c

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