lkml.org 
[lkml]   [2011]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectTTY loosing data with u_serial gadget
Hi all

I'm facing a problem with TTY loosing data using u_serial gadget.

I have a MPC8247 based system running 2.6.33. Data transfer is done
bidirectional
to see the problem more often. I use tty in raw mode and do some delayed
reads on the
receiver side to stress the throttling / un-throttling.

I tracked down the problem and was able to see that n_tty_receive_buf()
is not able to store all data in the read queue.

The function flush_to_ldisc() use the values of tty->receive_room to
schedule_delayed_work()or to pass data to receive_buf() and finally
n_tty_receive_buf().
Also the number of passed bytes rely on variable receive_room.

I added debug code to re-calculate the real free space.
left_space = N_TTY_BUF_SIZE - tty->read_cnt - 1;
Comparing receive_room and left_space showed in some cases big
differences up to 1600
bytes. left_space was always smaller and sometimes zero.
receive_room is calculated in n_tty_set_room() without read locking.
There are various "FIXME does n_tty_set_room need locking ?"
My test showed, adding a read looking solves the problem.

I asked me why is the locking not done? How big is the risk for dead-locks?
To minimize this risk, I reduced the changes to a minimum (see the patch
below).
The code in the patch only re-calculates the receive_room for raw mode
right before
it is used in flush_to_ldisc().

Can somebody give me an advice, what is the best way to solve this problem?

Thanks for your help.

Regards Stefan


From 91b04c875cecc890139ccdd101cfff6b02b5f22c Mon Sep 17 00:00:00 2001
From: Stefan Bigler <stefan.bigler@keymile.com>
Date: Wed, 16 Mar 2011 15:20:03 +0100
Subject: [PATCH 1/1] n_tty: fix race condition with receive_room

Do an additional update of receive_room with locking for raw tty.
The n_tty_set_room is done without locking what is known as a
potential problem.
In case of heavy load and raw tty and flush_to_ldisc() determine
the number of char to send with receive_buf(). If there is not
enough space available in receive_buf() the data is lost.
This additional update of receive_room should reduce the risk of
have invalid receive_room. It is not a clean fix but the risk of
risk a dead-lock with clean protection is too high

Signed-off-by: Stefan Bigler <stefan.bigler@keymile.com>
---
drivers/char/tty_buffer.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/char/tty_buffer.c b/drivers/char/tty_buffer.c
index f27c4d6..5db07fe 100644
--- a/drivers/char/tty_buffer.c
+++ b/drivers/char/tty_buffer.c
@@ -431,6 +431,22 @@ static void flush_to_ldisc(struct work_struct *work)
line discipline as we want to empty the queue */
if (test_bit(TTY_FLUSHPENDING, &tty->flags))
break;
+
+ /* Take read_lock to update receive_room.
+ This avoids invalid free space information.
+ To limit the risk of introducing problems
+ only do it for raw tty */
+ if (tty->raw) {
+ unsigned long flags_r;
+ spin_lock_irqsave(&tty->read_lock,
+ flags_r);
+ tty->receive_room =
+ N_TTY_BUF_SIZE -
+ tty->read_cnt - 1;
+ spin_unlock_irqrestore(&tty->read_lock,
+ flags_r);
+ }
+
if (!tty->receive_room) {
schedule_delayed_work(&tty->buf.work, 1);
break;
--
1.7.0.5





\
 
 \ /
  Last update: 2011-03-16 22:25    [from the cache]
©2003-2011 Jasper Spaans