Re: [PATCH] Fast Userspace Mutexes III.

[Robert Love <rml@tech9.net>]

On Sun, 2002-03-03 at 22:55, Rusty Russell wrote:
> 1) Use mmap/mprotect bits, not new syscall (thanks RTH, Erik Biederman)
> 2) Fix wakeup race in kernel (thanks Martin Wirth, Paul Mackerras)
> 3) Simplify locking to a single atomic (no more arch specifics!)
> 4) Use wake-one by handcoding queues.
> 5) Comments added.
> 
> Thanks to all for feedback and review: I'd appreciate a comment from
> those arch's which need to do something with the PROT_SEM bit.
> 
> Once again, tested on 2.4.18 UP PPC, compiles on 2.5.6-pre1.
> 
> Bad news is that we're up to 206 lines again.
> Rusty.

Good work.  I likee.

I have a couple comments and question:

> +static spinlock_t futex_lock = SPIN_LOCK_UNLOCKED;

Could we make this per-waitqueue?

> +asmlinkage int sys_futex(void *uaddr, int op)
< [...]
> +	switch (op) {
> +	case 1:
> +		ret = futex_up(head, page_address(page) + pos_in_page);
> +		break;
> +	case -1:

We should do:

	#define FUTEX_UP	1
	#define FUTEX_DOWN	-1

and put them in a common header (i.e. include/linux so both the kernel
and glibc will use it) and use that in our code and the kernel code. 
Just a finishing detail ...

> +static inline int __update_count(atomic_t *count, int incr)
> +{
> +	int old_count, tmp;
> +
> +	/* preempt_disable() */
> +	old_count = atomic_read(count);
> +	tmp = old_count > 0 ? old_count : 0;
> +	atomic_set(count, tmp + incr);
> +	return old_count;
> +}

You will want to do:

	int old_count, tmp;

	preempt_disable();
	old count = atomic_read(count);
	tmp = old_count > 0 ? old_count : 0;
	atomic_set(count, tmp + incr);
	preempt_enable();

	return old_count;

here.  The preempt statements compile away if CONFIG_PREEMPT is not set,
so you can just put them in, even on arches that don't do preemption
yet.

... oh, and I would love an example of using it in userspace ;)

Nice work, Rusty.

	Robert Love