Re: [Devel] [PATCH hci-8.1 2/2] ve/vtty: wait for a closing CT tty instead of failing the console attach

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] tty: vtty_open_master() busy-waits with bare schedule() for closing tty
--

commit ddd1463f95114c5d89bc233ad67329d19bb74551
Author: Denis V. Lunev <den@openvz.org>

ve/vtty: wait for a closing CT tty instead of failing the console attach

When the last CT-side close has committed to the final close,
vtty_open_master() used to return -EBUSY. With this patch it drops its
locks, calls schedule() and looks the pair up again. A pending signal ends
the wait with -ERESTARTSYS.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-152365

> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c

[ ... ]

>  	if (!created && slave->count == 0) {
> -		ret = -EBUSY;
> -		goto err_unlock;
> +		ret = -ERESTARTSYS;
> +		if (signal_pending(current))
> +			goto err_unlock;
> +		tty_unlock(slave);
> +		vtty_drop_context();
> +		mutex_unlock(&tty_mutex);
> +		schedule();
> +		goto retry;
>  	}

[Severity: Low]
Is a bare schedule() the right way to wait here? The task is still
TASK_RUNNING, and nothing sets the task state or waits on an event. In
that case schedule() only yields the CPU, and the loop becomes a busy-wait.

The closer can sleep for a while before it reaches vtty_shutdown():

tty_release_struct()
    tty_ldisc_release(tty);
    tty_flush_works(tty);
    ...
    mutex_lock(&tty_mutex);
    release_tty(tty, idx);

While it does this, vtty_open_master() keeps looping. Each pass does the
following:

    mutex_lock(&tty_mutex);
    ...
    mutex_unlock(&tty_mutex);
    schedule();

So the opener can use up to a full CPU and keeps contending with the closer
for tty_mutex. If the opener has a higher priority (for example SCHED_FIFO
or SCHED_RR) and runs on the same CPU as the closer, can it starve the
closer, so that the loop doesn't end until a signal arrives?

The commit message says the spinning matches tty_open(). Would a short
sleep, for example msleep_interruptible(), or an interruptible wait for the
pair to leave the vtty map be more robust here?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261102171504.3809155-1-den@openvz.org?part=2

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel