All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kirill Tkhai <ktkhai@odin.com>
To: <linux-kernel@vger.kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	Michal Hocko <mhocko@suse.cz>, "Rik van Riel" <riel@redhat.com>,
	Ionut Alexa <ionut.m.alexa@gmail.com>,
	Peter Hurley <peter@hurleysoftware.com>,
	Kirill Tkhai <tkhai@yandex.ru>
Subject: [PATCH RFC 13/13] core: Nest tasklist_lock into task_struct::kin_lock
Date: Mon, 25 May 2015 20:46:17 +0300	[thread overview]
Message-ID: <1432575977.6866.46.camel@odin.com> (raw)
In-Reply-To: <20150525162722.5171.15901.stgit@pro>

tasklist_lock is global lock, and it should be locked as small time
as possible. So that, lets nest it under kin_lock to minimize this
time.

Signed-off-by: Kirill Tkhai <ktkhai@odin.com>
---
 fs/exec.c     |   20 ++++++++++----------
 kernel/exit.c |   13 +++++++------
 kernel/fork.c |   15 +++++++--------
 kernel/sys.c  |   17 ++++++++---------
 4 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 4de7770..0b44752 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -931,8 +931,7 @@ static int de_thread(struct task_struct *tsk)
 
 		for (;;) {
 			threadgroup_change_begin(tsk);
-			write_lock_irq(&tasklist_lock);
-			write_real_parent_lock(tsk);
+			write_real_parent_lock_irq(tsk);
 			/*
 			 * Do this under real_parent's kin_lock to ensure
 			 * that exit_notify() can't miss ->group_exit_task.
@@ -941,8 +940,7 @@ static int de_thread(struct task_struct *tsk)
 			if (likely(leader->exit_state))
 				break;
 			__set_current_state(TASK_KILLABLE);
-			write_real_parent_unlock(tsk);
-			write_unlock_irq(&tasklist_lock);
+			write_real_parent_unlock_irq(tsk);
 			threadgroup_change_end(tsk);
 			schedule();
 			if (unlikely(__fatal_signal_pending(tsk)))
@@ -964,6 +962,8 @@ static int de_thread(struct task_struct *tsk)
 
 		BUG_ON(!same_thread_group(leader, tsk));
 		BUG_ON(has_group_leader_pid(tsk));
+
+		write_lock(&tasklist_lock);
 		/*
 		 * An exec() starts a new thread group with the
 		 * TGID of the previous thread group. Rehash the
@@ -982,6 +982,7 @@ static int de_thread(struct task_struct *tsk)
 		transfer_pid(leader, tsk, PIDTYPE_SID);
 
 		list_replace_rcu(&leader->tasks, &tsk->tasks);
+		write_unlock(&tasklist_lock);
 		list_replace_init(&leader->sibling, &tsk->sibling);
 
 		tsk->group_leader = tsk;
@@ -1003,8 +1004,7 @@ static int de_thread(struct task_struct *tsk)
 		 */
 		if (unlikely(leader->ptrace))
 			__wake_up_parent(leader, leader->parent);
-		write_real_parent_unlock(tsk);
-		write_unlock_irq(&tasklist_lock);
+		write_real_parent_unlock_irq(tsk);
 		threadgroup_change_end(tsk);
 
 		release_task(leader);
@@ -1034,13 +1034,13 @@ static int de_thread(struct task_struct *tsk)
 		memcpy(newsighand->action, oldsighand->action,
 		       sizeof(newsighand->action));
 
-		write_lock_irq(&tasklist_lock);
-		write_parent_and_real_parent_lock(tsk);
+		write_parent_and_real_parent_lock_irq(tsk);
+		write_lock(&tasklist_lock);
 		spin_lock(&oldsighand->siglock);
 		rcu_assign_pointer(tsk->sighand, newsighand);
 		spin_unlock(&oldsighand->siglock);
-		write_parent_and_real_parent_unlock(tsk);
-		write_unlock_irq(&tasklist_lock);
+		write_unlock(&tasklist_lock);
+		write_parent_and_real_parent_unlock_irq(tsk);
 
 		__cleanup_sighand(oldsighand);
 	}
diff --git a/kernel/exit.c b/kernel/exit.c
index 99d7aa3..44641aa 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -179,9 +179,9 @@ void release_task(struct task_struct *p)
 
 	proc_flush_task(p);
 
-	write_lock_irq(&tasklist_lock);
-	write_parent_and_real_parent_lock(p);
+	write_parent_and_real_parent_lock_irq(p);
 	ptrace_release_task(p);
+	write_lock(&tasklist_lock);
 	__exit_signal(p);
 	write_unlock(&tasklist_lock);
 
@@ -642,11 +642,12 @@ static void exit_notify(struct task_struct *tsk, int group_dead)
 
 	forget_original_parent(tsk, &dead);
 
-	read_lock_irq(&tasklist_lock);
-	write_parent_and_real_parent_lock(tsk);
-	if (group_dead)
+	write_parent_and_real_parent_lock_irq(tsk);
+	if (group_dead) {
+		read_lock(&tasklist_lock);
 		kill_orphaned_pgrp(tsk->group_leader, NULL);
-	read_unlock(&tasklist_lock);
+		read_unlock(&tasklist_lock);
+	}
 
 	if (unlikely(tsk->ptrace)) {
 		int sig = thread_group_leader(tsk) &&
diff --git a/kernel/fork.c b/kernel/fork.c
index 63e225b..b7cb176 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1520,19 +1520,18 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	 * Make it visible to the rest of the system, but dont wake it up yet.
 	 * Need tasklist lock for parent etc handling!
 	 */
-	write_lock_irq(&tasklist_lock);
-
 	/* CLONE_PARENT re-uses the old parent */
 	if (clone_flags & (CLONE_PARENT|CLONE_THREAD)) {
-		write_real_parent_lock(current);
+		write_real_parent_lock_irq(current);
 		p->real_parent = current->real_parent;
 		p->parent_exec_id = current->parent_exec_id;
 	} else {
-		write_lock(&current->kin_lock);
+		write_lock_irq(&current->kin_lock);
 		p->real_parent = current;
 		p->parent_exec_id = current->self_exec_id;
 	}
 
+	write_lock(&tasklist_lock);
 	spin_lock(&current->sighand->siglock);
 
 	/*
@@ -1552,8 +1551,8 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	recalc_sigpending();
 	if (signal_pending(current)) {
 		spin_unlock(&current->sighand->siglock);
-		write_real_parent_unlock(p);
-		write_unlock_irq(&tasklist_lock);
+		write_unlock(&tasklist_lock);
+		write_real_parent_unlock_irq(p);
 		retval = -ERESTARTNOINTR;
 		goto bad_fork_free_pid;
 	}
@@ -1594,9 +1593,9 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 
 	total_forks++;
 	spin_unlock(&current->sighand->siglock);
-	write_real_parent_unlock(p);
+	write_unlock(&tasklist_lock);
+	write_real_parent_unlock_irq(p);
 	syscall_tracepoint_update(p);
-	write_unlock_irq(&tasklist_lock);
 
 	proc_fork_connector(p);
 	cgroup_post_fork(p);
diff --git a/kernel/sys.c b/kernel/sys.c
index 783aec4..013be3e 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -931,17 +931,16 @@ SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
 		return -EINVAL;
 	rcu_read_lock();
 
-	/* From this point forward we keep holding onto the tasklist lock
-	 * so that our parent does not change from under us. -DaveM
-	 */
-	write_lock_irq(&tasklist_lock);
-
 	err = -ESRCH;
 	p = find_task_by_vpid(pid);
 	if (!p)
 		goto out2;
 
-	write_real_parent_lock(p);
+	write_real_parent_lock_irq(p);
+	if (p->flags & PF_EXITPIDONE)
+		goto out3;
+
+	write_lock(&tasklist_lock);
 
 	err = -EINVAL;
 	if (!thread_group_leader(p))
@@ -983,10 +982,10 @@ SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
 
 	err = 0;
 out:
-	/* All (almost) paths lead to here, thus we are safe. -DaveM */
-	write_real_parent_unlock(p);
+	write_unlock(&tasklist_lock);
+out3:
+	write_real_parent_unlock_irq(p);
 out2:
-	write_unlock_irq(&tasklist_lock);
 	rcu_read_unlock();
 	return err;
 }




      parent reply	other threads:[~2015-05-25 17:46 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20150525162722.5171.15901.stgit@pro>
2015-05-25 17:44 ` [PATCH RFC 01/13] exit: Clarify choice of new parent in forget_original_parent() Kirill Tkhai
2015-05-25 17:44 ` [PATCH RFC 02/13] rwlock_t: Implement double_write_{,un}lock() Kirill Tkhai
2015-05-25 17:44 ` [PATCH RFC 03/13] pid_ns: Implement rwlock_t pid_ns::cr_lock for locking child_reaper Kirill Tkhai
2015-05-25 17:44 ` [PATCH RFC 04/13] exit: Small refactoring mm_update_next_owner() Kirill Tkhai
2015-05-25 17:45 ` [PATCH RFC 05/13] fs: Refactoring in get_children_pid() Kirill Tkhai
2015-05-25 17:45 ` [PATCH RFC 06/13] core: Add rwlock_t task_list::kin_lock Kirill Tkhai
2015-05-25 17:45 ` [PATCH RFC 07/13] kin_lock: Implement helpers for kin_lock locking Kirill Tkhai
2015-05-25 17:45 ` [PATCH RFC 08/13] core: Use kin_lock synchronizations between parent and child and for thread group Kirill Tkhai
2015-05-25 17:45 ` [PATCH RFC 09/13] exit: Use for_each_thread() in do_wait() Kirill Tkhai
2015-05-26 19:46   ` Oleg Nesterov
2015-05-27  9:33     ` Kirill Tkhai
2015-05-27  9:42       ` Kirill Tkhai
2015-05-25 17:45 ` [PATCH RFC 10/13] exit: Add struct wait_opts's member held_lock and use it for tasklist_lock Kirill Tkhai
2015-05-25 17:46 ` [PATCH RFC 11/13] exit: Syncronize on kin_lock while do_notify_parent() Kirill Tkhai
2015-05-25 17:46 ` [PATCH RFC 12/13] exit: Delete write dependence on tasklist_lock in exit_notify() Kirill Tkhai
2015-05-25 17:46 ` Kirill Tkhai [this message]

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=1432575977.6866.46.camel@odin.com \
    --to=ktkhai@odin.com \
    --cc=akpm@linux-foundation.org \
    --cc=ionut.m.alexa@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@suse.cz \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peter@hurleysoftware.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=tkhai@yandex.ru \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.