lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 11 Mar 2019 09:23:13 +0100
From:   Alexander Sverdlin <alexander.sverdlin@...il.com>
To:     Oleksij Rempel <o.rempel@...gutronix.de>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>,
        Steven Walter <stevenrwalter@...il.com>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/3] drivers/tty: convert tty_port to use
 kthread_worker

Hello Oleksij,

On Thu, 10 Jan 2019 11:12:31 +0100
Oleksij Rempel <o.rempel@...gutronix.de> wrote:

> From: Steven Walter <stevenrwalter@...il.com>
> 
> Use kthread_worker instead of workqueues.  For now there is only a
> single workqueue, but the intention is to bring back the "low_latency"
> tty option, along with a second high-priority kthread worker.
> 
> Even without a second worker this patch allows to give a higher priority
> to tty processing by modifying the priority of the corresponding
> kthread.
> 
> Signed-off-by: Steven Walter <stevenrwalter@...il.com>
> Tested-by: Oleksij Rempel <o.rempel@...gutronix.de>

Tested-by: Alexander Sverdlin <alexander.sverdlin@...il.com>

In general I agree with Linus, that a thread with some random
priority is suboptimal, and we actually should move this
work into the context of the receiving thread, to properly
inherit its priority. But this would be quite amount of rework.

In the meanwhile this patch series restores the "low_latency"
tty option. Thanks for your efforts!

> ---
>  drivers/tty/tty_buffer.c | 21 ++++++++++++++-------
>  drivers/tty/tty_io.c     |  1 +
>  include/linux/tty.h      |  7 ++++---
>  3 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index e0090e65d83a..18bd7f48a319 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <linux/types.h>
> +#include <linux/kthread.h>
>  #include <linux/errno.h>
>  #include <linux/tty.h>
>  #include <linux/tty_driver.h>
> @@ -497,7 +498,7 @@ receive_buf(struct tty_port *port, struct tty_buffer *head, int count)
>   *		 'consumer'
>   */
>  
> -static void flush_to_ldisc(struct work_struct *work)
> +static void flush_to_ldisc(struct kthread_work *work)
>  {
>  	struct tty_port *port = container_of(work, struct tty_port, buf.work);
>  	struct tty_bufhead *buf = &port->buf;
> @@ -557,10 +558,16 @@ void tty_flip_buffer_push(struct tty_port *port)
>  }
>  EXPORT_SYMBOL(tty_flip_buffer_push);
>  
> +static DEFINE_KTHREAD_WORKER(tty_buffer_worker);
> +
>  bool tty_buffer_queue_work(struct tty_port *port)
>  {
> -	struct tty_bufhead *buf = &port->buf;
> -	return schedule_work(&buf->work);
> +	return kthread_queue_work(&tty_buffer_worker, &port->buf.work);
> +}
> +
> +void tty_buffer_init_kthread(void)
> +{
> +	kthread_run(kthread_worker_fn, &tty_buffer_worker, "tty");
>  }
>  
>  /**
> @@ -582,7 +589,7 @@ void tty_buffer_init(struct tty_port *port)
>  	init_llist_head(&buf->free);
>  	atomic_set(&buf->mem_used, 0);
>  	atomic_set(&buf->priority, 0);
> -	INIT_WORK(&buf->work, flush_to_ldisc);
> +	kthread_init_work(&buf->work, flush_to_ldisc);
>  	buf->mem_limit = TTYB_DEFAULT_MEM_LIMIT;
>  }
>  
> @@ -614,12 +621,12 @@ bool tty_buffer_restart_work(struct tty_port *port)
>  	return tty_buffer_queue_work(port);
>  }
>  
> -bool tty_buffer_cancel_work(struct tty_port *port)
> +void tty_buffer_cancel_work(struct tty_port *port)
>  {
> -	return cancel_work_sync(&port->buf.work);
> +	tty_buffer_flush_work(port);
>  }
>  
>  void tty_buffer_flush_work(struct tty_port *port)
>  {
> -	flush_work(&port->buf.work);
> +	kthread_flush_work(&port->buf.work);
>  }
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index bfe9ad85b362..5fd7cecbe4e7 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -3476,6 +3476,7 @@ void console_sysfs_notify(void)
>   */
>  int __init tty_init(void)
>  {
> +	tty_buffer_init_kthread();
>  	cdev_init(&tty_cdev, &tty_fops);
>  	if (cdev_add(&tty_cdev, MKDEV(TTYAUX_MAJOR, 0), 1) ||
>  	    register_chrdev_region(MKDEV(TTYAUX_MAJOR, 0), 1, "/dev/tty") < 0)
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 226a9eff0766..924c1093967e 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -3,9 +3,9 @@
>  #define _LINUX_TTY_H
>  
>  #include <linux/fs.h>
> +#include <linux/kthread.h>
>  #include <linux/major.h>
>  #include <linux/termios.h>
> -#include <linux/workqueue.h>
>  #include <linux/tty_driver.h>
>  #include <linux/tty_ldisc.h>
>  #include <linux/mutex.h>
> @@ -84,7 +84,7 @@ static inline char *flag_buf_ptr(struct tty_buffer *b, int ofs)
>  
>  struct tty_bufhead {
>  	struct tty_buffer *head;	/* Queue head */
> -	struct work_struct work;
> +	struct kthread_work work;
>  	struct mutex	   lock;
>  	atomic_t	   priority;
>  	struct tty_buffer sentinel;
> @@ -510,8 +510,9 @@ extern void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld);
>  extern bool tty_buffer_queue_work(struct tty_port *port);
>  extern void tty_buffer_init(struct tty_port *port);
>  extern void tty_buffer_set_lock_subclass(struct tty_port *port);
> +extern void tty_buffer_init_kthread(void);
>  extern bool tty_buffer_restart_work(struct tty_port *port);
> -extern bool tty_buffer_cancel_work(struct tty_port *port);
> +extern void tty_buffer_cancel_work(struct tty_port *port);
>  extern void tty_buffer_flush_work(struct tty_port *port);
>  extern speed_t tty_termios_baud_rate(struct ktermios *termios);
>  extern speed_t tty_termios_input_baud_rate(struct ktermios *termios);


-- 
Alexander Sverdlin.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ