linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yi Wang <wang.yi59@zte.com.cn>
To: tglx@linutronix.de
Cc: mingo@redhat.com, peterz@infradead.org, dvhart@infradead.org,
	linux-kernel@vger.kernel.org, xue.zhihong@zte.com.cn,
	wang.yi59@zte.com.cn, jiang.xuexin@zte.com.cn,
	Yang Tao <yang.tao172@zte.com.cn>
Subject: [PATCH] Robust-futex wakes up the waiters will be missed
Date: Fri, 1 Nov 2019 10:03:09 +0800	[thread overview]
Message-ID: <1572573789-16557-1-git-send-email-wang.yi59@zte.com.cn> (raw)

From: Yang Tao <yang.tao172@zte.com.cn>

We found two scenarios:
(1)When the owner of a mutex lock with robust attribute and no_pi
will release the lock and executes pthread_mutex_unlock(),it is
killed after setting mutex->__data.__lock = 0 and before wake
others. It will go through the robust process, but it will not wake
up watiers because mutex->__data.__lock = 0.

                OWNER

        pthread_mutex_unlock()
                |
                |
                V
        atomic_exchange_rel (&mutex->__data.__lock, 0)
                        <------------------------killed
            lll_futex_wake ()                   |
                                                |
                                                |(__lock = 0)
                                                |(enter kernel)
                                                |
                                                V
                                            do_exit()
                                            exit_mm()
                                          mm_release()
                                        exit_robust_list()
                                        handle_futex_death()
                                                |
                                                |(__lock = 0)
                                                |(uval = 0)
                                                |
                                                V
        if ((uval & FUTEX_TID_MASK) != task_pid_vnr(curr))
                return 0;<------wakes up waiters will be missed

(2) When a waiter wakes up and returns to the user space, it is
killed before getting the lock (before modifying
mutex->__data.__lock), and other waiters will not wake up.

        OWNER                         WAITER

   pthread_mutex_unlock()
                |
                |(__lock = 0)
                |
                V
         futex_wake()
                                fuet_wait()   //awaked
                                        |
                                        |
                                        |(enter userspace)
                                        |(__lock = 0)
                                        |
                                        V
                        oldval = mutex->__data.__lock
                                          <-----------------killed
    atomic_compare_and_exchange_val_acq (&mutex->__data.__lock,  |
                        id | assume_other_futex_waiters, 0)      |
                                                                 |
                                                                 |
                                                   (enter kernel)|
                                                                 |
                                                                 V
                                                         do_exit()
                                                        |
                                                        |
                                                        V
                                        handle_futex_death()
                                        |
                                        |(__lock = 0)
                                        |(uval = 0)
                                        |
                                        V
        if ((uval & FUTEX_TID_MASK) != task_pid_vnr(curr))
                return 0;<------wakes up waiters will be missed
So, in these scenarios, task will not wake up waiters

We found that when the task was killed in two scenarios,
task->robust_list->list_op_pending =&mutex->__data.__list.__next,
so we can do something.

We think that task should wake up once when the following conditions
are met:
        (1) task->robust_list->list_op_pending != NULL;
        (2) mutex->__data.__lock = 0;
        (3) no_pi
In some cases, this may lead to some redundant wakeups, which will
reduce the efficiency of the program, but it will not affectthe
program operation, and it is very rare to meet these three
conditions.

At the same time, we only wake up and do not set the died bit,
because mutex->__data.__lock = 0; mutex->__data.__owner = 0;
At this time, it can be seen that there is no owner,and the wake-up
process directly take the lock.

If the died bit is set, it may cause some misoperation. Such as a
waiter being killed when the owner is releasing the lock, it will
mark the lock with the died bit, which is not good.

We don't need to set "mutex->__data.__count"(in mutex structure),
which will not affect repeated lock holding.

Signed-off-by: Yang Tao <yang.tao172@zte.com.cn>
---
 kernel/futex.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index bd18f60..c2fb590 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -3456,7 +3456,9 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
  * Process a futex-list entry, check whether it's owned by the
  * dying task, and do notification if so:
  */
-static int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int pi)
+static int handle_futex_death(u32 __user *uaddr,
+			      struct task_struct *curr, int pi,
+			      bool pending)
 {
 	u32 uval, uninitialized_var(nval), mval;
 	int err;
@@ -3469,6 +3471,10 @@ static int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int p
 	if (get_user(uval, uaddr))
 		return -1;
 
+	/* Robust-futex wakes up the waiters will be missed*/
+	if (pending && !pi && uval == 0)
+		futex_wake(uaddr, 1, 1, FUTEX_BITSET_MATCH_ANY);
+
 	if ((uval & FUTEX_TID_MASK) != task_pid_vnr(curr))
 		return 0;
 
@@ -3590,7 +3596,7 @@ void exit_robust_list(struct task_struct *curr)
 		 */
 		if (entry != pending)
 			if (handle_futex_death((void __user *)entry + futex_offset,
-						curr, pi))
+						curr, pi, 0))
 				return;
 		if (rc)
 			return;
@@ -3607,7 +3613,7 @@ void exit_robust_list(struct task_struct *curr)
 
 	if (pending)
 		handle_futex_death((void __user *)pending + futex_offset,
-				   curr, pip);
+				   curr, pip, 1);
 }
 
 long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
@@ -3784,7 +3790,7 @@ void compat_exit_robust_list(struct task_struct *curr)
 		if (entry != pending) {
 			void __user *uaddr = futex_uaddr(entry, futex_offset);
 
-			if (handle_futex_death(uaddr, curr, pi))
+			if (handle_futex_death(uaddr, curr, pi, 0))
 				return;
 		}
 		if (rc)
@@ -3803,7 +3809,7 @@ void compat_exit_robust_list(struct task_struct *curr)
 	if (pending) {
 		void __user *uaddr = futex_uaddr(pending, futex_offset);
 
-		handle_futex_death(uaddr, curr, pip);
+		handle_futex_death(uaddr, curr, pip, 1);
 	}
 }
 
-- 
2.15.2


             reply	other threads:[~2019-11-01  2:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-01  2:03 Yi Wang [this message]
2019-11-04  9:16 ` [PATCH] Robust-futex wakes up the waiters will be missed Peter Zijlstra
2019-11-06  3:23 Yi Wang
2019-11-06 19:02 ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1572573789-16557-1-git-send-email-wang.yi59@zte.com.cn \
    --to=wang.yi59@zte.com.cn \
    --cc=dvhart@infradead.org \
    --cc=jiang.xuexin@zte.com.cn \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=xue.zhihong@zte.com.cn \
    --cc=yang.tao172@zte.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).