Re: 2.4.27 Potential race in n_tty.c:write_chan()
From: Ryan Reading (rreading_at_msm.umr.edu)
Date: 12/06/04
- Previous message: H. Peter Anvin: "Re: [2.6 patch] i386/x86_64 msr.c: make two functions static"
- In reply to: Paul Fulghum: "Re: 2.4.27 Potential race in n_tty.c:write_chan()"
- Next in thread: Paul Fulghum: "Re: 2.4.27 Potential race in n_tty.c:write_chan()"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
To: linux-kernel@vger.kernel.org Date: Sun, 05 Dec 2004 23:42:22 -0500
On Sun, 2004-12-05 at 17:40, Paul Fulghum wrote:
> On Sun, 2004-12-05 at 15:54 -0500, Ryan Reading wrote:
> > So when write_chan() calls usb_driver::write(), typically the driver
> > calls usb_submit_urb(). The write() call then returns immediately
> > indicating that all of the data has been written (assuming it is
less
> > than the USB packets size). The driver however is still waiting for
an
> > interrupt to complete the write and wakeup the current kernel path.
If
> > write_chan() is called again and the interrupt is received within
the
> > window I outlined above, the current_state will be reset to
TASK_RUNNING
> > before the next usb_driver::write() is ever called! If this
happens, it
> > seems that we would lose synchronisity and potentially lock the
kernel
> > path.
>
> The line discipline write routine is serialized
> on a per tty basis in do_tty_write() of tty_io.c
> using tty->atomic_write semaphore, so you will not
> reenter write_chan() for a particular tty instance.
Per tty instance this is true, but this is not the case for multiple
userland::write() calls.
> Even if this were not the case, if the task state
> changes to TASK_RUNNING inside the window
> you describe, the only thing that happens is the loop
> executes again. The driver must decide if it can accept
> more data or not and return the appropriate value.
>
> There is no potential for deadlock.
Yes this does seem to be true. However, it can throw a driver into a
wierd state if this happens. Ideally the driver should be able to
recover. I'm not sure if we can guarantee that user_land protocol could
recover though, especially if write()s aren't guaranteed to complete
successfully.
> > It is also my understanding that the usb interrupt is generated from
the
> > ACK/NAK of the original usb_submit_urb(). If the driver is
returning
> > immediately without waiting on the interrupt and schedule() is never
> > being called, there is no guarantee that the write() happened
> > successfully (although we return that it has). It seems if a driver
> > wanted to guarantee this, it would have to artificially wait of the
> > interrupt before returning.
>
> True, but this is a matter of layering.
>
> The line discipline knows nothing about the driver's concept
> of write completion apart from the driver's write method
> return value. If it is critical for the write not to complete
> until the URB is sent, it is up to the driver to block
> and return the appropriate return value.
I guess my biggest concern here is the definition of the
tty_driver::write() interface. Should a driver be able to rely on the
schedule() call to complete its write? It seems that since the driver
is responsible for waking up the tty->write_wait, it should be able to
rely on the schedule() call. However in this implementation it cannot
which I'm sure is for performance reasons. Because of this, I don't
think the USB layer should be interacting with the tty layer in this
fashion.
So for this implementation of write_chan(), the tty_driver::write()
interface should note that the driver needs only to wakeup the
tty->write_wait iff it does not write all of the requested data in this
call. Is this a fair assessment and can this be depended on for future
implementations?
Anyway, part of this discussion needs to happen on the USB mailing list
since the usb-skeleton driver outlines this method of interacting with
the tty layer. Given the current implementation I don't think the
usb-skeleton is correct.
Thanks for your help.
-- Ryan
> --
> Paul Fulghum
> paulkf@microgate.com
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- Previous message: H. Peter Anvin: "Re: [2.6 patch] i386/x86_64 msr.c: make two functions static"
- In reply to: Paul Fulghum: "Re: 2.4.27 Potential race in n_tty.c:write_chan()"
- Next in thread: Paul Fulghum: "Re: 2.4.27 Potential race in n_tty.c:write_chan()"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
|
