linux-kernel - Re: [PATCH v3] tty: tty_port: add workqueue to flip tty buffer

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] [day] [month] [year] [list]

Message-Id: <20251125021959.1509256-1-jackzxcui1989@163.com>
Date: Tue, 25 Nov 2025 10:19:59 +0800
From: Xin Zhao <jackzxcui1989@....com>
To: tj@...nel.org,
	gregkh@...uxfoundation.org,
	jirislaby@...nel.org
Cc: hch@...radead.org,
	jackzxcui1989@....com,
	linux-kernel@...r.kernel.org,
	linux-serial@...r.kernel.org
Subject: Re: [PATCH v3] tty: tty_port: add workqueue to flip tty buffer

On Mon, Nov 24, 2025 at 06:02:07AM -1000 Tejun Heo <tj@...nel.org> wrote:

> > On Mon, Nov 24, 2025 at 01:17:51PM +0800, Xin Zhao wrote:
> > > > +/**
> > > > + * tty_flip_wq_init -- prepare workqueue for tty/pty flip buffer work
> > > > + * @port: tty_port of the device
> > > > + * @driver: tty_driver for this device
> > > > + * @index: index of the tty
> > > > + *
> > > > + * Not all tty_port will be initialized by tty_port_init where tty_flip_wq will
> > > > + * be set to system_unbound_wq as default. Allocate workqueue with WQ_SYSFS for
> > > > + * flip buffer, so that cpumask and nice can be changed dynamically.
> > > > + */
> > > > +void tty_flip_wq_init(struct tty_port *port, struct tty_driver *driver,
> > > > +		      unsigned int index)
> > > > +{
> > > > +	char name[64];
> > > > +
> > > > +	if (driver->type == TTY_DRIVER_TYPE_PTY) {
> > > > +		port->buf.tty_flip_wq = pty_flip_wq;
> > > > +		return;
> > > > +	}
> > > > +	tty_line_name(driver, index, name);
> > > > +	if (!port->buf.tty_flip_wq
> > > > +		|| port->buf.tty_flip_wq == system_unbound_wq) {
> > > > +		port->buf.tty_flip_wq = alloc_workqueue("%s-flip-wq",
> > > > +							WQ_UNBOUND | WQ_SYSFS,
> > > > +							0, name);
> > > > +		if (unlikely(!port->buf.tty_flip_wq))
> > > > +			port->buf.tty_flip_wq = system_unbound_wq;
> > > > +	}
> > > > +}
> > 
> > These look overly elaborate to me. Just fail init if workqueue allocation
> > fails? Also, is it necessary for each port to have separate port?
> 
> Sorry, I meant a separate workqueue.

I admit it is a bit complex.
I searched for a long time and found in function tty_port_link_device seems to be an
appropriate place to initialize the workqueue. However, the tty_port_link_device
interface does not return a value, so error handling seems to be quite difficult.

Greg KH suggested creating a workqueue for each tty port, as follow:
> >  
> >  	tty_flip_buffer_commit(buf->tail);
> > -	queue_work(system_unbound_wq, &buf->work);
> > +	queue_work(port->wq_tty_flip ?: system_unbound_wq, &buf->work);
> 
> Why not just do this for all tty ports?  What is the benifit of keeping
> this on the system_unbound_wq for all other tty devices?

I think if there are many tty devices on the system, having separate workqueues
might be the right approach. Or should a single tty_driver share one?

Greg and Jiri Slaby have any comments on this?

--
Xin Zhao