Re: [PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

From: Ilya Zykov
Date: Thu Nov 22 2012 - 14:00:04 EST


On 22.11.2012 8:29, Ilya Zykov wrote:
On 22.11.2012 4:47, andrew mcgregor wrote:


On 11/22/2012 at 10:39 AM, in message <50AD4A01.7060500@xxxxxxx>,
Ilya Zykov
<ilya@xxxxxxx> wrote:
On 22.11.2012 1:30, Alan Cox wrote:
Function reset_buffer_flags() also invoked during the
ioctl(...,TCFLSH,..). At the time of request we can have full buffers
and throttled driver too. If we don't unthrottle driver, we can get
forever throttled driver, because after request, we will have
empty buffers and throttled driver and there is no place to unthrottle
driver.
It simple reproduce with "pty" pair then one side sleep on
tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do
writers wake up.


So instead of revertng it why not just fix it ? Just add an argument to
the reset_buffer_flags function to indicate if unthrottling is
permitted.

Alan

Because in my opinion, unthrottling permitted always, except release
last filp (tty->count == 0)

Maybe so, but the patch was there in the first place to resolve an
actual observed bug, where a driver would lock up. So the behaviour
needs preserved.

Andrew


Maybe it was wrong driver, unfortunately, I didn't find full information
about this bug. As an example, if driver indirectly call
reset_buffer_flags() in driver's close() function it will be before
decrement last (tty->count).



Particularly, many drivers and 'serial_core.c' use tty_ldisc_flush() in
own close() function. tty_ldisc_flush() call reset_buffer_flags() indirectly.
I think is wrong way use tty_ldisc_flush() in driver's close() function, because tty layer 'tty_release()' call tty_ldisc_release() after decremented (tty->count), and clear all buffers.
We don't care about this in driver. And call ldisc's function.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/