[2/3] tty: Avoid threads files iterations in __do_SAK()

Submitted by Kirill Tkhai on Jan. 18, 2018, 9:48 a.m.

Details

Message ID 151626892421.1120.6368421061544912462.stgit@localhost.localdomain
State New
Series "Series without cover letter"
Headers show

Commit Message

Kirill Tkhai Jan. 18, 2018, 9:48 a.m.
The patch makes __do_SAK() iterate a next thread files
only in case of the thread's files are different
to previous. I.e., if all threads points the same
files_struct, the files will be iterated only once.

Since all threads have the same files_struct is
the generic case for most Linux systems, this
improvement should clearly speed up __do_SAK()
execution.

Also, for_each_process()/for_each_thread() are
used instead of do_each_thread()/while_each_thread().
This prepares __do_SAK() to become tasklist_lock
free, and will be made in next patch.

https://jira.sw.ru/browse/PSBM-80340

Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 drivers/tty/tty_io.c |   39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b46b357a3b76..50b3170e091c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3006,7 +3006,8 @@  void __do_SAK(struct tty_struct *tty)
 #ifdef TTY_SOFT_SAK
 	tty_hangup(tty);
 #else
-	struct task_struct *g, *p;
+	struct task_struct *p, *t;
+	struct files_struct *files;
 	struct pid *session;
 	int		i;
 
@@ -3029,24 +3030,36 @@  void __do_SAK(struct tty_struct *tty)
 	/* Now kill any processes that happen to have the
 	 * tty open.
 	 */
-	do_each_thread(g, p) {
+	for_each_process(p) {
 		if (p->signal->tty == tty) {
 			printk(KERN_NOTICE "SAK: killed process %d"
 			    " (%s): task_session(p)==tty->session\n",
 			    task_pid_nr(p), p->comm);
-			send_sig(SIGKILL, p, 1);
-			continue;
+			goto kill;
 		}
-		task_lock(p);
-		i = iterate_fd(p->files, 0, this_tty, tty);
-		if (i != 0) {
-			printk(KERN_NOTICE "SAK: killed process %d"
-			    " (%s): fd#%d opened to the tty\n",
-				    task_pid_nr(p), p->comm, i - 1);
-			send_sig(SIGKILL, p, 1);
+
+		files = NULL;
+		for_each_thread(p, t) {
+			if (t->files == files) /* racy but we do not care */
+				continue;
+
+			task_lock(t);
+			files = t->files;
+			i = iterate_fd(files, 0, this_tty, tty);
+			task_unlock(t);
+
+			if (i != 0) {
+				printk(KERN_NOTICE "SAK: killed process %d"
+					" (%s): fd#%d opened to the tty\n",
+					task_pid_nr(p), p->comm, i - 1);
+				goto kill;
+			}
 		}
-		task_unlock(p);
-	} while_each_thread(g, p);
+
+		continue;
+kill:
+		send_sig(SIGKILL, p, 1);
+	}
 	qread_unlock(&tasklist_lock);
 #endif
 }

Comments

Pavel Tikhomirov Jan. 18, 2018, 2:45 p.m.
Please see one comment below, everything else looks good.

> @@ -3029,24 +3030,36 @@ void __do_SAK(struct tty_struct *tty)
>   	/* Now kill any processes that happen to have the
>   	 * tty open.
>   	 */
> -	do_each_thread(g, p) {
> +	for_each_process(p) {
>   		if (p->signal->tty == tty) {
>   			printk(KERN_NOTICE "SAK: killed process %d"
>   			    " (%s): task_session(p)==tty->session\n",
>   			    task_pid_nr(p), p->comm);
> -			send_sig(SIGKILL, p, 1);
> -			continue;
> +			goto kill;
>   		}
> -		task_lock(p);
> -		i = iterate_fd(p->files, 0, this_tty, tty);
> -		if (i != 0) {
> -			printk(KERN_NOTICE "SAK: killed process %d"
> -			    " (%s): fd#%d opened to the tty\n",
> -				    task_pid_nr(p), p->comm, i - 1);
> -			send_sig(SIGKILL, p, 1);
> +
> +		files = NULL;
> +		for_each_thread(p, t) {
> +			if (t->files == files) /* racy but we do not care */
> +				continue;
> +
> +			task_lock(t);
> +			files = t->files;
> +			i = iterate_fd(files, 0, this_tty, tty);
> +			task_unlock(t);
> +
> +			if (i != 0) {
> +				printk(KERN_NOTICE "SAK: killed process %d"
> +					" (%s): fd#%d opened to the tty\n",
> +					task_pid_nr(p), p->comm, i - 1);
> +				goto kill;

We skip all threads which go after "t" if we jump to the label, it seem 
we need a separate "kill" for these loop.

> +			}
>   		}
> -		task_unlock(p);
> -	} while_each_thread(g, p);
> +
> +		continue;
> +kill:
> +		send_sig(SIGKILL, p, 1);
> +	}
>   	qread_unlock(&tasklist_lock);
>   #endif
>   }
>