[PATCH -tip 0/2] futex: Fault/error injection capabilities

[PATCH -tip 0/2] futex: Fault/error injection capabilities

[Davidlohr Bueso]

Hello,

I've been wanting this for a while to improve overall futex
testing. I must say that I checked rather late when I was
nearly finished with patch 2 if something similar for futexes
had already been proposed. Sure enough, in 2009 this was discussed[1].

Coincidently, I also took the natural approach of making use of
our fault-injection machinery. I have no idea if perf nowadays does
such things, and if it does not, I honestly don't have the bandwidth
to do it Ingo's preferred way -- when there is nothing wrong with
this approach, imho (0 overhead). Anyway, here's a working patch.

Patch 1 is merely a trivial add-on.

[1]: https://lwn.net/Articles/364742/

Thanks!

Davidlohr Bueso (2):
  futex: Enhance comments in futex_lock_pi() for blocking paths
  futex: Fault/error injection capabilities

 Documentation/fault-injection/fault-injection.txt |  11 +++
 kernel/futex.c                                    | 100 +++++++++++++++++++++-
 lib/Kconfig.debug                                 |   7 ++
 3 files changed, 114 insertions(+), 4 deletions(-)

-- 
2.1.4

[PATCH 1/2] futex: Enhance comments in futex_lock_pi() for blocking paths

[Davidlohr Bueso]

... serves a bit better to clarify between blocking
and non-blocking code paths.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 kernel/futex.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index c4a182f..153eb22 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -2268,8 +2268,11 @@ static long futex_wait_restart(struct restart_block *restart)
 /*
  * Userspace tried a 0 -> TID atomic transition of the futex value
  * and failed. The kernel side here does the whole locking operation:
- * if there are waiters then it will block, it does PI, etc. (Due to
- * races the kernel might see a 0 value of the futex too.)
+ * if there are waiters then it will block as a consequence of relying
+ * on rt-mutexes, it does PI, etc. (Due to races the kernel might see
+ * a 0 value of the futex too.).
+ *
+ * Also serves as futex trylock_pi()'ing, and due semantics.
  */
 static int futex_lock_pi(u32 __user *uaddr, unsigned int flags,
 			 ktime_t *time, int trylock)
@@ -2300,6 +2303,10 @@ retry_private:
 
 	ret = futex_lock_pi_atomic(uaddr, hb, &q.key, &q.pi_state, current, 0);
 	if (unlikely(ret)) {
+		/*
+		 * Atomic work succeeded and we got the lock,
+		 * or failed. Either way, we do _not_ block.
+		 */
 		switch (ret) {
 		case 1:
 			/* We got the lock. */
-- 
2.1.4

[tip:locking/core] futex: Enhance comments in futex_lock_pi() for blocking paths

[tip-bot for Davidlohr Bueso]

Commit-ID:  767f509ca11269c2bcd92e3972a93096f2173ac0
Gitweb:     http://git.kernel.org/tip/767f509ca11269c2bcd92e3972a93096f2173ac0
Author:     Davidlohr Bueso <dave@stgolabs.net>
AuthorDate: Mon, 29 Jun 2015 23:26:01 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 20 Jul 2015 11:45:45 +0200

futex: Enhance comments in futex_lock_pi() for blocking paths

... serves a bit better to clarify between blocking
and non-blocking code paths.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Darren Hart <darren@dvhart.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Link: http://lkml.kernel.org/r/1435645562-975-2-git-send-email-dave@stgolabs.net
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/futex.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index c4a182f..153eb22 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -2268,8 +2268,11 @@ static long futex_wait_restart(struct restart_block *restart)
 /*
  * Userspace tried a 0 -> TID atomic transition of the futex value
  * and failed. The kernel side here does the whole locking operation:
- * if there are waiters then it will block, it does PI, etc. (Due to
- * races the kernel might see a 0 value of the futex too.)
+ * if there are waiters then it will block as a consequence of relying
+ * on rt-mutexes, it does PI, etc. (Due to races the kernel might see
+ * a 0 value of the futex too.).
+ *
+ * Also serves as futex trylock_pi()'ing, and due semantics.
  */
 static int futex_lock_pi(u32 __user *uaddr, unsigned int flags,
 			 ktime_t *time, int trylock)
@@ -2300,6 +2303,10 @@ retry_private:
 
 	ret = futex_lock_pi_atomic(uaddr, hb, &q.key, &q.pi_state, current, 0);
 	if (unlikely(ret)) {
+		/*
+		 * Atomic work succeeded and we got the lock,
+		 * or failed. Either way, we do _not_ block.
+		 */
 		switch (ret) {
 		case 1:
 			/* We got the lock. */

Re: [tip:locking/core] futex: Enhance comments in futex_lock_pi() for blocking paths

[Darren Hart]

On Mon, Jul 20, 2015 at 03:57:37AM -0700, tip-bot for Davidlohr Bueso wrote:
> Commit-ID:  767f509ca11269c2bcd92e3972a93096f2173ac0
> Gitweb:     http://git.kernel.org/tip/767f509ca11269c2bcd92e3972a93096f2173ac0
> Author:     Davidlohr Bueso <dave@stgolabs.net>
> AuthorDate: Mon, 29 Jun 2015 23:26:01 -0700
> Committer:  Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Mon, 20 Jul 2015 11:45:45 +0200
> 
> futex: Enhance comments in futex_lock_pi() for blocking paths
> 
> ... serves a bit better to clarify between blocking
> and non-blocking code paths.
> 
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Darren Hart <darren@dvhart.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Link: http://lkml.kernel.org/r/1435645562-975-2-git-send-email-dave@stgolabs.net
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  kernel/futex.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/futex.c b/kernel/futex.c
> index c4a182f..153eb22 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -2268,8 +2268,11 @@ static long futex_wait_restart(struct restart_block *restart)
>  /*
>   * Userspace tried a 0 -> TID atomic transition of the futex value
>   * and failed. The kernel side here does the whole locking operation:
> - * if there are waiters then it will block, it does PI, etc. (Due to
> - * races the kernel might see a 0 value of the futex too.)
> + * if there are waiters then it will block as a consequence of relying
> + * on rt-mutexes, it does PI, etc. (Due to races the kernel might see
> + * a 0 value of the futex too.).
> + *
> + * Also serves as futex trylock_pi()'ing, and due semantics.

Hrm, what does 'due' mean here?

-- 
Darren Hart
Intel Open Source Technology Center

Re: [tip:locking/core] futex: Enhance comments in futex_lock_pi() for blocking paths

[Davidlohr Bueso]

On Mon, 2015-07-20 at 14:24 -0700, Darren Hart wrote:
> On Mon, Jul 20, 2015 at 03:57:37AM -0700, tip-bot for Davidlohr Bueso wrote:
> > Commit-ID:  767f509ca11269c2bcd92e3972a93096f2173ac0
> > Gitweb:     http://git.kernel.org/tip/767f509ca11269c2bcd92e3972a93096f2173ac0
> > Author:     Davidlohr Bueso <dave@stgolabs.net>
> > AuthorDate: Mon, 29 Jun 2015 23:26:01 -0700
> > Committer:  Thomas Gleixner <tglx@linutronix.de>
> > CommitDate: Mon, 20 Jul 2015 11:45:45 +0200
> > 
> > futex: Enhance comments in futex_lock_pi() for blocking paths
> > 
> > ... serves a bit better to clarify between blocking
> > and non-blocking code paths.
> > 
> > Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Darren Hart <darren@dvhart.com>
> > Cc: Davidlohr Bueso <dave@stgolabs.net>
> > Link: http://lkml.kernel.org/r/1435645562-975-2-git-send-email-dave@stgolabs.net
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > ---
> >  kernel/futex.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/futex.c b/kernel/futex.c
> > index c4a182f..153eb22 100644
> > --- a/kernel/futex.c
> > +++ b/kernel/futex.c
> > @@ -2268,8 +2268,11 @@ static long futex_wait_restart(struct restart_block *restart)
> >  /*
> >   * Userspace tried a 0 -> TID atomic transition of the futex value
> >   * and failed. The kernel side here does the whole locking operation:
> > - * if there are waiters then it will block, it does PI, etc. (Due to
> > - * races the kernel might see a 0 value of the futex too.)
> > + * if there are waiters then it will block as a consequence of relying
> > + * on rt-mutexes, it does PI, etc. (Due to races the kernel might see
> > + * a 0 value of the futex too.).
> > + *
> > + * Also serves as futex trylock_pi()'ing, and due semantics.
> 
> Hrm, what does 'due' mean here?

By that I meant respective/corresponding.

Re: [tip:locking/core] futex: Enhance comments in futex_lock_pi() for blocking paths

[Darren Hart]

On Mon, Jul 20, 2015 at 02:59:04PM -0700, Davidlohr Bueso wrote:
> On Mon, 2015-07-20 at 14:24 -0700, Darren Hart wrote:
> > On Mon, Jul 20, 2015 at 03:57:37AM -0700, tip-bot for Davidlohr Bueso wrote:
> > > Commit-ID:  767f509ca11269c2bcd92e3972a93096f2173ac0
> > > Gitweb:     http://git.kernel.org/tip/767f509ca11269c2bcd92e3972a93096f2173ac0
> > > Author:     Davidlohr Bueso <dave@stgolabs.net>
> > > AuthorDate: Mon, 29 Jun 2015 23:26:01 -0700
> > > Committer:  Thomas Gleixner <tglx@linutronix.de>
> > > CommitDate: Mon, 20 Jul 2015 11:45:45 +0200
> > > 
> > > futex: Enhance comments in futex_lock_pi() for blocking paths
> > > 
> > > ... serves a bit better to clarify between blocking
> > > and non-blocking code paths.
> > > 
> > > Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Darren Hart <darren@dvhart.com>
> > > Cc: Davidlohr Bueso <dave@stgolabs.net>
> > > Link: http://lkml.kernel.org/r/1435645562-975-2-git-send-email-dave@stgolabs.net
> > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > > ---
> > >  kernel/futex.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/kernel/futex.c b/kernel/futex.c
> > > index c4a182f..153eb22 100644
> > > --- a/kernel/futex.c
> > > +++ b/kernel/futex.c
> > > @@ -2268,8 +2268,11 @@ static long futex_wait_restart(struct restart_block *restart)
> > >  /*
> > >   * Userspace tried a 0 -> TID atomic transition of the futex value
> > >   * and failed. The kernel side here does the whole locking operation:
> > > - * if there are waiters then it will block, it does PI, etc. (Due to
> > > - * races the kernel might see a 0 value of the futex too.)
> > > + * if there are waiters then it will block as a consequence of relying
> > > + * on rt-mutexes, it does PI, etc. (Due to races the kernel might see
> > > + * a 0 value of the futex too.).
> > > + *
> > > + * Also serves as futex trylock_pi()'ing, and due semantics.
> > 
> > Hrm, what does 'due' mean here?
> 
> By that I meant respective/corresponding.

Right, OK, read awkwardly to me for some reason.

-- 
Darren Hart
Intel Open Source Technology Center

[PATCH 2/2] futex: Fault/error injection capabilities

[Davidlohr Bueso]

Although futexes are well known for being a royal pita,
we really have very little debugging capabilities - except
for relying on tglx's eye half the time.

By simply making use of the existing fault-injection machinery,
we can improve this situation, allowing generating artificial
uaddress faults and deadlock scenarios. Of course, when this is
disabled in production systems, the overhead for failure checks
is practically zero -- so this is very cheap at the same time.
Future work would be nice to now enhance trinity to make use of
this.

There is a special tunable 'ignore-private', which can filter
out private futexes. Given the tsk->make_it_fail filter and
this option, pi futexes can be narrowed down pretty closely.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 Documentation/fault-injection/fault-injection.txt | 11 +++
 kernel/futex.c                                    | 89 ++++++++++++++++++++++-
 lib/Kconfig.debug                                 |  7 ++
 3 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt
index 4cf1a2a..415484f 100644
--- a/Documentation/fault-injection/fault-injection.txt
+++ b/Documentation/fault-injection/fault-injection.txt
@@ -15,6 +15,10 @@ o fail_page_alloc
 
   injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
 
+o fail_futex
+
+  injects futex deadlock and uaddr fault errors.
+
 o fail_make_request
 
   injects disk IO errors on devices permitted by setting
@@ -113,6 +117,12 @@ configuration of fault-injection capabilities.
 	specifies the minimum page allocation order to be injected
 	failures.
 
+- /sys/kernel/debug/fail_futex/ignore-private:
+
+	Format: { 'Y' | 'N' }
+	default is 'N', setting it to 'Y' will disable failure injections
+	when dealing with private (address space) futexes.
+
 o Boot option
 
 In order to inject faults while debugfs is not available (early boot time),
@@ -121,6 +131,7 @@ use the boot option:
 	failslab=
 	fail_page_alloc=
 	fail_make_request=
+	fail_futex=
 	mmc_core.fail_request=<interval>,<probability>,<space>,<times>
 
 How to add new fault injection capability
diff --git a/kernel/futex.c b/kernel/futex.c
index 153eb22..6ea31bb 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -64,6 +64,7 @@
 #include <linux/hugetlb.h>
 #include <linux/freezer.h>
 #include <linux/bootmem.h>
+#include <linux/fault-inject.h>
 
 #include <asm/futex.h>
 
@@ -258,6 +259,66 @@ static unsigned long __read_mostly futex_hashsize;
 
 static struct futex_hash_bucket *futex_queues;
 
+/*
+ * Fault injections for futexes.
+ */
+#ifdef CONFIG_FAIL_FUTEX
+
+static struct {
+	struct fault_attr attr;
+
+	u32 ignore_private;
+} fail_futex = {
+	.attr = FAULT_ATTR_INITIALIZER,
+	.ignore_private = 0,
+};
+
+static int __init setup_fail_futex(char *str)
+{
+	return setup_fault_attr(&fail_futex.attr, str);
+}
+__setup("fail_futex=", setup_fail_futex);
+
+bool should_fail_futex(bool fshared)
+{
+	if (fail_futex.ignore_private && !fshared)
+		return false;
+
+	return should_fail(&fail_futex.attr, 1);
+}
+
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+
+static int __init fail_futex_debugfs(void)
+{
+	umode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
+	struct dentry *dir;
+
+	dir = fault_create_debugfs_attr("fail_futex", NULL,
+					&fail_futex.attr);
+	if (IS_ERR(dir))
+		return PTR_ERR(dir);
+
+	if (!debugfs_create_bool("ignore-private", mode, dir,
+				 &fail_futex.ignore_private)) {
+		debugfs_remove_recursive(dir);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+late_initcall(fail_futex_debugfs);
+
+#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
+
+#else
+static inline bool should_fail_futex(bool fshared)
+{
+	return false;
+}
+#endif /* CONFIG_FAIL_FUTEX */
+
 static inline void futex_get_mm(union futex_key *key)
 {
 	atomic_inc(&key->private.mm->mm_count);
@@ -413,6 +474,9 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
 	if (unlikely(!access_ok(rw, uaddr, sizeof(u32))))
 		return -EFAULT;
 
+	if (unlikely(should_fail_futex(fshared)))
+		return -EFAULT;
+
 	/*
 	 * PROCESS_PRIVATE futexes are fast.
 	 * As the mm cannot disappear under us and the 'key' only needs
@@ -428,6 +492,10 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
 	}
 
 again:
+	/* Ignore any VERIFY_READ mapping (futex common case) */
+	if (unlikely(should_fail_futex(fshared)))
+		return -EFAULT;
+
 	err = get_user_pages_fast(address, 1, 1, &page);
 	/*
 	 * If write access is not required (eg. FUTEX_WAIT), try
@@ -516,7 +584,7 @@ again:
 		 * A RO anonymous page will never change and thus doesn't make
 		 * sense for futex operations.
 		 */
-		if (ro) {
+		if (unlikely(should_fail_futex(fshared)) || ro) {
 			err = -EFAULT;
 			goto out;
 		}
@@ -974,6 +1042,9 @@ static int lock_pi_update_atomic(u32 __user *uaddr, u32 uval, u32 newval)
 {
 	u32 uninitialized_var(curval);
 
+	if (unlikely(should_fail_futex(true)))
+		return -EFAULT;
+
 	if (unlikely(cmpxchg_futex_value_locked(&curval, uaddr, uval, newval)))
 		return -EFAULT;
 
@@ -1015,12 +1086,18 @@ static int futex_lock_pi_atomic(u32 __user *uaddr, struct futex_hash_bucket *hb,
 	if (get_futex_value_locked(&uval, uaddr))
 		return -EFAULT;
 
+	if (unlikely(should_fail_futex(true)))
+		return -EFAULT;
+
 	/*
 	 * Detect deadlocks.
 	 */
 	if ((unlikely((uval & FUTEX_TID_MASK) == vpid)))
 		return -EDEADLK;
 
+	if ((unlikely(should_fail_futex(true))))
+		return -EDEADLK;
+
 	/*
 	 * Lookup existing state first. If it exists, try to attach to
 	 * its pi_state.
@@ -1155,6 +1232,9 @@ static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this,
 	 */
 	newval = FUTEX_WAITERS | task_pid_vnr(new_owner);
 
+	if (unlikely(should_fail_futex(true)))
+		ret = -EFAULT;
+
 	if (cmpxchg_futex_value_locked(&curval, uaddr, uval, newval))
 		ret = -EFAULT;
 	else if (curval != uval)
@@ -1457,6 +1537,9 @@ static int futex_proxy_trylock_atomic(u32 __user *pifutex,
 	if (get_futex_value_locked(&curval, pifutex))
 		return -EFAULT;
 
+	if (unlikely(should_fail_futex(true)))
+		return -EFAULT;
+
 	/*
 	 * Find the top_waiter and determine if there are additional waiters.
 	 * If the caller intends to requeue more than 1 waiter to pifutex,
@@ -2537,7 +2620,7 @@ int handle_early_requeue_pi_wakeup(struct futex_hash_bucket *hb,
  * futex_wait_requeue_pi() - Wait on uaddr and take uaddr2
  * @uaddr:	the futex we initially wait on (non-pi)
  * @flags:	futex flags (FLAGS_SHARED, FLAGS_CLOCKRT, etc.), they must be
- * 		the same type, no requeueing from private to shared, etc.
+ *		the same type, no requeueing from private to shared, etc.
  * @val:	the expected value of uaddr
  * @abs_time:	absolute timeout
  * @bitset:	32 bit wakeup bitset set by userspace, defaults to all
@@ -3012,6 +3095,8 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
 	if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
 		      cmd == FUTEX_WAIT_BITSET ||
 		      cmd == FUTEX_WAIT_REQUEUE_PI)) {
+		if (unlikely(should_fail_futex(!(op & FUTEX_PRIVATE_FLAG))))
+			return -EFAULT;
 		if (copy_from_user(&ts, utime, sizeof(ts)) != 0)
 			return -EFAULT;
 		if (!timespec_valid(&ts))
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b908048..e772be6 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1537,6 +1537,13 @@ config FAIL_MMC_REQUEST
 	  and to test how the mmc host driver handles retries from
 	  the block device.
 
+config FAIL_FUTEX
+	bool "Fault-injection capability for futexes"
+	select DEBUG_FS
+	depends on FAULT_INJECTION && FUTEX
+	help
+	  Provide fault-injection capability for futexes.
+
 config FAULT_INJECTION_DEBUG_FS
 	bool "Debugfs entries for fault-injection capabilities"
 	depends on FAULT_INJECTION && SYSFS && DEBUG_FS
-- 
2.1.4

[tip:locking/core] futex: Fault/error injection capabilities

[tip-bot for Davidlohr Bueso]

Commit-ID:  ab51fbab39d864f3223e44a2600fd951df261f0b
Gitweb:     http://git.kernel.org/tip/ab51fbab39d864f3223e44a2600fd951df261f0b
Author:     Davidlohr Bueso <dave@stgolabs.net>
AuthorDate: Mon, 29 Jun 2015 23:26:02 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 20 Jul 2015 11:45:45 +0200

futex: Fault/error injection capabilities

Although futexes are well known for being a royal pita,
we really have very little debugging capabilities - except
for relying on tglx's eye half the time.

By simply making use of the existing fault-injection machinery,
we can improve this situation, allowing generating artificial
uaddress faults and deadlock scenarios. Of course, when this is
disabled in production systems, the overhead for failure checks
is practically zero -- so this is very cheap at the same time.
Future work would be nice to now enhance trinity to make use of
this.

There is a special tunable 'ignore-private', which can filter
out private futexes. Given the tsk->make_it_fail filter and
this option, pi futexes can be narrowed down pretty closely.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Darren Hart <darren@dvhart.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Link: http://lkml.kernel.org/r/1435645562-975-3-git-send-email-dave@stgolabs.net
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/fault-injection/fault-injection.txt | 11 +++
 kernel/futex.c                                    | 89 ++++++++++++++++++++++-
 lib/Kconfig.debug                                 |  7 ++
 3 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt
index 4cf1a2a..415484f 100644
--- a/Documentation/fault-injection/fault-injection.txt
+++ b/Documentation/fault-injection/fault-injection.txt
@@ -15,6 +15,10 @@ o fail_page_alloc
 
   injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
 
+o fail_futex
+
+  injects futex deadlock and uaddr fault errors.
+
 o fail_make_request
 
   injects disk IO errors on devices permitted by setting
@@ -113,6 +117,12 @@ configuration of fault-injection capabilities.
 	specifies the minimum page allocation order to be injected
 	failures.
 
+- /sys/kernel/debug/fail_futex/ignore-private:
+
+	Format: { 'Y' | 'N' }
+	default is 'N', setting it to 'Y' will disable failure injections
+	when dealing with private (address space) futexes.
+
 o Boot option
 
 In order to inject faults while debugfs is not available (early boot time),
@@ -121,6 +131,7 @@ use the boot option:
 	failslab=
 	fail_page_alloc=
 	fail_make_request=
+	fail_futex=
 	mmc_core.fail_request=<interval>,<probability>,<space>,<times>
 
 How to add new fault injection capability
diff --git a/kernel/futex.c b/kernel/futex.c
index 153eb22..6ea31bb 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -64,6 +64,7 @@
 #include <linux/hugetlb.h>
 #include <linux/freezer.h>
 #include <linux/bootmem.h>
+#include <linux/fault-inject.h>
 
 #include <asm/futex.h>
 
@@ -258,6 +259,66 @@ static unsigned long __read_mostly futex_hashsize;
 
 static struct futex_hash_bucket *futex_queues;
 
+/*
+ * Fault injections for futexes.
+ */
+#ifdef CONFIG_FAIL_FUTEX
+
+static struct {
+	struct fault_attr attr;
+
+	u32 ignore_private;
+} fail_futex = {
+	.attr = FAULT_ATTR_INITIALIZER,
+	.ignore_private = 0,
+};
+
+static int __init setup_fail_futex(char *str)
+{
+	return setup_fault_attr(&fail_futex.attr, str);
+}
+__setup("fail_futex=", setup_fail_futex);
+
+bool should_fail_futex(bool fshared)
+{
+	if (fail_futex.ignore_private && !fshared)
+		return false;
+
+	return should_fail(&fail_futex.attr, 1);
+}
+
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+
+static int __init fail_futex_debugfs(void)
+{
+	umode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
+	struct dentry *dir;
+
+	dir = fault_create_debugfs_attr("fail_futex", NULL,
+					&fail_futex.attr);
+	if (IS_ERR(dir))
+		return PTR_ERR(dir);
+
+	if (!debugfs_create_bool("ignore-private", mode, dir,
+				 &fail_futex.ignore_private)) {
+		debugfs_remove_recursive(dir);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+late_initcall(fail_futex_debugfs);
+
+#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
+
+#else
+static inline bool should_fail_futex(bool fshared)
+{
+	return false;
+}
+#endif /* CONFIG_FAIL_FUTEX */
+
 static inline void futex_get_mm(union futex_key *key)
 {
 	atomic_inc(&key->private.mm->mm_count);
@@ -413,6 +474,9 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
 	if (unlikely(!access_ok(rw, uaddr, sizeof(u32))))
 		return -EFAULT;
 
+	if (unlikely(should_fail_futex(fshared)))
+		return -EFAULT;
+
 	/*
 	 * PROCESS_PRIVATE futexes are fast.
 	 * As the mm cannot disappear under us and the 'key' only needs
@@ -428,6 +492,10 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
 	}
 
 again:
+	/* Ignore any VERIFY_READ mapping (futex common case) */
+	if (unlikely(should_fail_futex(fshared)))
+		return -EFAULT;
+
 	err = get_user_pages_fast(address, 1, 1, &page);
 	/*
 	 * If write access is not required (eg. FUTEX_WAIT), try
@@ -516,7 +584,7 @@ again:
 		 * A RO anonymous page will never change and thus doesn't make
 		 * sense for futex operations.
 		 */
-		if (ro) {
+		if (unlikely(should_fail_futex(fshared)) || ro) {
 			err = -EFAULT;
 			goto out;
 		}
@@ -974,6 +1042,9 @@ static int lock_pi_update_atomic(u32 __user *uaddr, u32 uval, u32 newval)
 {
 	u32 uninitialized_var(curval);
 
+	if (unlikely(should_fail_futex(true)))
+		return -EFAULT;
+
 	if (unlikely(cmpxchg_futex_value_locked(&curval, uaddr, uval, newval)))
 		return -EFAULT;
 
@@ -1015,12 +1086,18 @@ static int futex_lock_pi_atomic(u32 __user *uaddr, struct futex_hash_bucket *hb,
 	if (get_futex_value_locked(&uval, uaddr))
 		return -EFAULT;
 
+	if (unlikely(should_fail_futex(true)))
+		return -EFAULT;
+
 	/*
 	 * Detect deadlocks.
 	 */
 	if ((unlikely((uval & FUTEX_TID_MASK) == vpid)))
 		return -EDEADLK;
 
+	if ((unlikely(should_fail_futex(true))))
+		return -EDEADLK;
+
 	/*
 	 * Lookup existing state first. If it exists, try to attach to
 	 * its pi_state.
@@ -1155,6 +1232,9 @@ static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this,
 	 */
 	newval = FUTEX_WAITERS | task_pid_vnr(new_owner);
 
+	if (unlikely(should_fail_futex(true)))
+		ret = -EFAULT;
+
 	if (cmpxchg_futex_value_locked(&curval, uaddr, uval, newval))
 		ret = -EFAULT;
 	else if (curval != uval)
@@ -1457,6 +1537,9 @@ static int futex_proxy_trylock_atomic(u32 __user *pifutex,
 	if (get_futex_value_locked(&curval, pifutex))
 		return -EFAULT;
 
+	if (unlikely(should_fail_futex(true)))
+		return -EFAULT;
+
 	/*
 	 * Find the top_waiter and determine if there are additional waiters.
 	 * If the caller intends to requeue more than 1 waiter to pifutex,
@@ -2537,7 +2620,7 @@ int handle_early_requeue_pi_wakeup(struct futex_hash_bucket *hb,
  * futex_wait_requeue_pi() - Wait on uaddr and take uaddr2
  * @uaddr:	the futex we initially wait on (non-pi)
  * @flags:	futex flags (FLAGS_SHARED, FLAGS_CLOCKRT, etc.), they must be
- * 		the same type, no requeueing from private to shared, etc.
+ *		the same type, no requeueing from private to shared, etc.
  * @val:	the expected value of uaddr
  * @abs_time:	absolute timeout
  * @bitset:	32 bit wakeup bitset set by userspace, defaults to all
@@ -3012,6 +3095,8 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
 	if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
 		      cmd == FUTEX_WAIT_BITSET ||
 		      cmd == FUTEX_WAIT_REQUEUE_PI)) {
+		if (unlikely(should_fail_futex(!(op & FUTEX_PRIVATE_FLAG))))
+			return -EFAULT;
 		if (copy_from_user(&ts, utime, sizeof(ts)) != 0)
 			return -EFAULT;
 		if (!timespec_valid(&ts))
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index e2894b2..22554d6 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1542,6 +1542,13 @@ config FAIL_MMC_REQUEST
 	  and to test how the mmc host driver handles retries from
 	  the block device.
 
+config FAIL_FUTEX
+	bool "Fault-injection capability for futexes"
+	select DEBUG_FS
+	depends on FAULT_INJECTION && FUTEX
+	help
+	  Provide fault-injection capability for futexes.
+
 config FAULT_INJECTION_DEBUG_FS
 	bool "Debugfs entries for fault-injection capabilities"
 	depends on FAULT_INJECTION && SYSFS && DEBUG_FS

Re: [tip:locking/core] futex: Fault/error injection capabilities

[Dave Jones]

On Mon, Jul 20, 2015 at 03:57:58AM -0700, tip-bot for Davidlohr Bueso wrote:
 > Commit-ID:  ab51fbab39d864f3223e44a2600fd951df261f0b
 > Gitweb:     http://git.kernel.org/tip/ab51fbab39d864f3223e44a2600fd951df261f0b
 > Author:     Davidlohr Bueso <dave@stgolabs.net>
 > AuthorDate: Mon, 29 Jun 2015 23:26:02 -0700
 > Committer:  Thomas Gleixner <tglx@linutronix.de>
 > CommitDate: Mon, 20 Jul 2015 11:45:45 +0200
 > 
 > futex: Fault/error injection capabilities
 > 
 > Although futexes are well known for being a royal pita,
 > we really have very little debugging capabilities - except
 > for relying on tglx's eye half the time.
 > 
 > By simply making use of the existing fault-injection machinery,
 > we can improve this situation, allowing generating artificial
 > uaddress faults and deadlock scenarios. Of course, when this is
 > disabled in production systems, the overhead for failure checks
 > is practically zero -- so this is very cheap at the same time.
 > Future work would be nice to now enhance trinity to make use of
 > this.

If you enable any of the fault injection modules, ie, like you demonstrate here:

 > +- /sys/kernel/debug/fail_futex/ignore-private:
 > +
 > +	Format: { 'Y' | 'N' }
 > +	default is 'N', setting it to 'Y' will disable failure injections
 > +	when dealing with private (address space) futexes.
 > +

each pid gets a a make-it-fail file in /proc/self/.  If present, trinity will set this to 1
for child processes.

So it should work today unless I've missed something.

	Dave

Re: [PATCH -tip 0/2] futex: Fault/error injection capabilities

[Davidlohr Bueso]

Ingo, any thoughts about this?

Thanks.

On Mon, 2015-06-29 at 23:26 -0700, Davidlohr Bueso wrote:
> Hello,
> 
> I've been wanting this for a while to improve overall futex
> testing. I must say that I checked rather late when I was
> nearly finished with patch 2 if something similar for futexes
> had already been proposed. Sure enough, in 2009 this was discussed[1].
> 
> Coincidently, I also took the natural approach of making use of
> our fault-injection machinery. I have no idea if perf nowadays does
> such things, and if it does not, I honestly don't have the bandwidth
> to do it Ingo's preferred way -- when there is nothing wrong with
> this approach, imho (0 overhead). Anyway, here's a working patch.
> 
> Patch 1 is merely a trivial add-on.
> 
> [1]: https://lwn.net/Articles/364742/
> 
> Thanks!
> 
> Davidlohr Bueso (2):
>   futex: Enhance comments in futex_lock_pi() for blocking paths
>   futex: Fault/error injection capabilities
> 
>  Documentation/fault-injection/fault-injection.txt |  11 +++
>  kernel/futex.c                                    | 100 +++++++++++++++++++++-
>  lib/Kconfig.debug                                 |   7 ++
>  3 files changed, 114 insertions(+), 4 deletions(-)
> 

Re: [PATCH -tip 0/2] futex: Fault/error injection capabilities

[Thomas Gleixner]

On Mon, 13 Jul 2015, Davidlohr Bueso wrote:

> Ingo, any thoughts about this?

It's on my review list....