futex.c and EWOULDBLOCK vs. EAGAIN patch

Since PARISC is the only Linux architecture which has different values for EAGAIN and EWOULDBLOCK, we are running in various issues.

One of them is, that e.g. glibc checks for EAGAIN instead of EWOULDBLOCK (and vise versa) in nptl/pthread_mutex_trylock.c. This doesn't hurt other architectures, but it hurts parisc.

I was planning to send the patch below to linux kernel mailing list. But before I do it, I would like to get feedback from the kernel hackers here on the list.

What do you think?

Is this patch useful?

Or will people just call me an idiot if I ask if this patch could be applied? If you think it's useful, maybe another patch description would be more appropriate?

Any feedback is welcome...

Helge

-----------------------
[PATCH] futex.c: return EAGAIN instead of EWOULDBLOCK

Patch summary:
In the Linux kernel all architectures beside the PARISC architecture define EWOULDBLOCK as EAGAIN:

arch/sparc/include/asm/errno.h:#define  EWOULDBLOCK     EAGAIN  /* Operation would block */
arch/mips/include/asm/errno.h:#define   EWOULDBLOCK     EAGAIN  /* Operation would block */
arch/alpha/include/asm/errno.h:#define  EWOULDBLOCK     EAGAIN  /* Operation would block */
include/asm-generic/errno.h:#define     EWOULDBLOCK     EAGAIN  /* Operation would block */

The only exception is PARISC, which tries to keep HP-UX compatibility:
arch/parisc/include/asm/errno.h:#define EWOULDBLOCK 246 /* Operation would block (Linux returns EAGAIN) */

This difference leads to various problems with userspace, which often thinks that EWOULDBLOCK and EGAIN are of the same value. To avoid problems userspace sometimes has to check the return value for both values.

The following patch cleans up the Linux kernel futex implementation to always return EAGAIN instead of EWOULDBLOCK.
Binary compatibility for all architectures beside PARISC is still kept, but on PARISC this will ease life a lot.

Signed-off-by Helge Deller <[email protected]>

kernel/futex.c +8 -9

index e7a35f1..28d14ff 100644
@@ -1732,7 +1732,7 @@ static void futex_wait_queue_me(struct futex_hash_bucket *hb, struct futex_q *q,
  *
  * Returns:
  *  0 - uaddr contains val and hb has been locked
- * <1 - -EFAULT or -EWOULDBLOCK (uaddr does not contain val) and hb is unlcoked
+ * <1 - -EFAULT or -EAGAIN (uaddr does not contain val) and hb is unlcoked
  */
 static int futex_wait_setup(u32 __user *uaddr, u32 val, int fshared,
 			   struct futex_q *q, struct futex_hash_bucket **hb)

@@ -1784,7 +1784,7 @@ retry_private:
 
 	if (uval != val) {
 		queue_unlock(q, *hb);
-		ret = -EWOULDBLOCK;
+		ret = -EAGAIN;
 	}
 
 out:

@@ -1969,7 +1969,7 @@ retry_private:
 	else {
 		ret = rt_mutex_trylock(&q.pi_state->pi_mutex);
 		/* Fixup the trylock return value: */
-		ret = ret ? 0 : -EWOULDBLOCK;
+		ret = ret ? 0 : -EAGAIN;
 	}
 
 	spin_lock(q.lock_ptr);

@@ -2154,7 +2154,7 @@ int handle_early_requeue_pi_wakeup(struct futex_hash_bucket *hb,
 		plist_del(&q->list, &q->list.plist);
 
 		/* Handle spurious wakeups gracefully */
-		ret = -EWOULDBLOCK;
+		ret = -EAGAIN;
 		if (timeout && !timeout->task)
 			ret = -ETIMEDOUT;
 		else if (signal_pending(current))

@@ -2196,7 +2196,7 @@ int handle_early_requeue_pi_wakeup(struct futex_hash_bucket *hb,
  * 7) timeout
  * 8) other lock acquisition failure
  *
- * If 6, return -EWOULDBLOCK (restarting the syscall would do the same).
+ * If 6, return -EAGAIN (restarting the syscall would do the same).
  *
  * If 4 or 7, we cleanup and return with -ETIMEDOUT.
  *

@@ -2318,10 +2318,10 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, int fshared,
 		 * We've already been requeued, but cannot restart by calling
 		 * futex_lock_pi() directly. We could restart this syscall, but
 		 * it would detect that the user space "val" changed and return
-		 * -EWOULDBLOCK.  Save the overhead of the restart and return
-		 * -EWOULDBLOCK directly.
+		 * -EAGAIN.  Save the overhead of the restart and return
+		 * -EAGAIN directly.
 		 */
-		ret = -EWOULDBLOCK;
+		ret = -EAGAIN;
 	}
 
 out_put_keys:
--

To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 11 lines · Helge Deller

Since PARISC is the only Linux architecture which has different values for EAGAIN and EWOULDBLOCK, we are running in various issues.

One of them is, that e.g. glibc checks for EAGAIN instead of EWOULDBLOCK (and vise versa) in nptl/pthread_mutex_trylock.c. This doesn't hurt other architectures, but it hurts parisc.

I was planning to send the patch below to linux kernel mailing list. But before I do it, I would like to get feedback from the kernel hackers here on the list.

Quoted 1 line

What do you think?

Do it.
We kept them distinct to enable support for HPUX binaries. This is never going to happen.

Quoted 1 line

Is this patch useful?

Yes.

Quoted 2 lines

Or will people just call me an idiot if I ask if this patch could be applied? If you think it's useful, maybe another patch description would be more appropriate?

Could we just equate like other arcthitectures so the kernel only ever returns one of those and not both?

An application that checked only for EWOULDBLOCK might fail now that EAGAIN is being returned.

Is this a concern? I don't think it is. The POSIX standard, in all cases I could find, says the functions return "EAGAIN or EWOULDBLOCK", meaning that a conforming program must check for both.

It's a two instruction penalty(*) in the kernel syscall return path, just do it there. I've got a box at Red Hat with all the source to the whole damn world on it we run greps against to look for dumb stuff like memcpy bugs.

I'll toss a grep for EAGAIN and not EWOULDBLOCK and vice versa and
see how much stuff might b0rk.

We discussed this a couple years ago, and it ended up being a complete rats nest of error returns that needed fixing.

regards, Kyle.

*: CMPI,N(=), LDI, maybe a third since I don't remember how sign extension works off the top of my head.

Quoted 4 lines · Kyle McMartin

It's a two instruction penalty(*) in the kernel syscall return path, just do it there. I've got a box at Red Hat with all the source to the whole damn world on it we run greps against to look for dumb stuff like memcpy bugs.

Why look for it? Just change the kernel to not ever return one of them.

Quoted 2 lines

I'll toss a grep for EAGAIN and not EWOULDBLOCK and vice versa and
see how much stuff might b0rk.

Please do...I'd be curious to see which programs might be affected and if we care.

Quoted 3 lines

We discussed this a couple years ago, and it ended up being a complete rats nest of error returns that needed fixing.

Quoted 4 lines

regards, Kyle.

*: CMPI,N(=), LDI, maybe a third since I don't remember how sign extension works off the top of my head.

Mispredicted branches cost something like 20-25 cycles. I forgot now but I do know they aren't cheap.

I'd rather not add more hacks for this they are unlikely to ever be removed again. I'd rather have status quo than add some hack to the syscall return path.

Quoted 5 lines

Quoted 2 lines

*: CMPI,N(=), LDI, maybe a third since I don't remember how sign extension works off the top of my head.

Mispredicted branches cost something like 20-25 cycles. I forgot now but I do know they aren't cheap.

That's not a branch, Grant.

Quoted 3 lines

I'd rather not add more hacks for this they are unlikely to ever be removed again. I'd rather have status quo than add some hack to the syscall return path.

The alternative is to rebuild all of userspace, again.

If I generate a list of debian packages that look at EWOULDBLOCK or EAGAIN, couldn't we just regenerate those packages once the kernel change is in place and the kernel header files pushed to debian experimental? There can't be that many packages.

And of those that do, I believe most time the old binaries will generally work since EWOULDBLOCK code paths are unlikely to get exercised. Or is there evidence to the contrary?

Quoted 7 lines · Carlos O'Donell

An application that checked only for EWOULDBLOCK might fail now that EAGAIN is being returned.

Is this a concern? I don't think it is. The POSIX standard, in all cases I could find, says the functions return "EAGAIN or EWOULDBLOCK", meaning that a conforming program must check for both.

It's a two instruction penalty(*) in the kernel syscall return path, just do it there. I've got a box at Red Hat with all the source to the whole damn world on it we run greps against to look for dumb stuff like memcpy bugs. I'll toss a grep for EAGAIN and not EWOULDBLOCK and vice versa and see how much stuff might b0rk.

We discussed this a couple years ago, and it ended up being a complete rats nest of error returns that needed fixing.

Quoted 14 lines · Grant Grundler

Quoted 12 lines · Helge Deller

Since PARISC is the only Linux architecture which has different values for EAGAIN and EWOULDBLOCK, we are running in various issues.

One of them is, that e.g. glibc checks for EAGAIN instead of EWOULDBLOCK (and vise versa) in nptl/pthread_mutex_trylock.c. This doesn't hurt other architectures, but it hurts parisc.

I was planning to send the patch below to linux kernel mailing list. But before I do it, I would like to get feedback from the kernel hackers here on the list.

What do you think?

Quoted 3 lines

Do it.
We kept them distinct to enable support for HPUX binaries. This is never going to happen.

Grant, does this mean you endorse purging the kernel of all uses of EWOULDBLOCK (with the exception of the header file used by userspace)?

FWIW I think this is the right approach.

Quoted 10 lines

Quoted 2 lines

Is this patch useful?

Yes.

Quoted 2 lines

Or will people just call me an idiot if I ask if this patch could be applied? If you think it's useful, maybe another patch description would be more appropriate?

Could we just equate like other arcthitectures so the kernel only ever returns one of those and not both?

There are ABI issues that I would like to discuss before you send your patch upstream.

Once you change the kernel EWOULDBLOCK to be equal to EAGAIN, you have broken the ABI.

A kernel syscall that used to return EWOULDBLOCK, now returns EAGAIN.

An application that checked only for EWOULDBLOCK might fail now that EAGAIN is being returned.

Is this a concern? I don't think it is. The POSIX standard, in all cases I could find, says the functions return "EAGAIN or EWOULDBLOCK", meaning that a conforming program must check for both.

The alternative to Helge's patch is a glibc patch to check the return of *every* syscall and convert EWOULDBLOCK to EAGAIN. This approach has a cost to our syscall fast path. Therefore I am not as inclined to follow this approach.

Cheers,
Carlos.

Since PARISC is the only Linux architecture which has different values for EAGAIN and EWOULDBLOCK, we are running in various issues.

One of them is, that e.g. glibc checks for EAGAIN instead of EWOULDBLOCK (and vise versa) in nptl/pthread_mutex_trylock.c. This doesn't hurt other architectures, but it hurts parisc.