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

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Nov 25, 2025 at 10:19:59AM +0800, Xin Zhao wrote:
> On Mon, Nov 24, 2025 at 06:02:07AM -1000 Tejun Heo <tj@kernel.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?

I don't know, what have you found in your testing?  Will multiple
workqueues cause too much overhead for large multi-port systems (i.e.
modem banks?)

Perhaps start with just a single workqueue and then increase it if there
is contention later if people notice?

thanks,

greg k-h