On Thu, Jul 13, 2006 at 10:20:19PM +0400, Sergei Organov wrote: > Alan Cox <[EMAIL PROTECTED]> writes: > > On Iau, 2006-07-13 at 18:17 +0400, Sergei Organov wrote: > >> This problem may occur with any tty driver that doesn't stop to insert > >> data into the tty buffers in throttled state. And yes, there are such > >> drivers in the tree. Before Paul's changes, the tty just dropped bytes > >> that aren't accepted by ldisc, so this problem had no chances to arise. > > > > You must honour throttle. > > I do, it's Greg who doesn't ;) BTW, isn't it OK to just check for > tty->throttled where appropriate if I don't have anything special to do > at unthrottled/throttled transition time? > > > That has always been the case. At all times you should attempt to > > homour tty->receive_room and also ->throttle. If you don't it breaks. > > Yes, but the difference is what "it" actually is. Loosing some > characters at some rare or "might never in fact happen conditions" is > one "it", and exhausting kernel memory at (even more) rare conditions is > a different "it", isn't it? > > Besides, if the throttle() is that important and failure to handle it is > a big mistake, why is it optional then? I mean why struct tty_operations > with throttle field set to NULL is accepted in the first place? The same > question is applicable to the struct usb_serial_driver.
Yes, I didn't realize it was required. If it is, we should add this change. > >> latter cases drivers that insert too much data without pushing to ldisc > >> may cause similar problem. Anyway, you definitely know better what to do > >> about it. > > > > Might be a good idea to put a limiter in before 2.6.18 proper just to > > trap any other drivers that have that bug. At least printk a warning and > > refuse the allocation once there is say 64K queued. That way the driver > > author gets a hint all is not well. > > I'm afraid that the limit won't work well as a hint for driver > developers that didn't honour throttle, as real applications do usually > read from the files they open, and therefore the problem most probably > won't be noticed for a long time. > > Provided the limiter is put, why not to make it a variable with 64K > default? Driver writers that for whatever reason decide they need more > in buffers will be able to change that, but then it will be their > deliberate decision, not just underestimation of consequences of failure > to handle throttle() due to a lack of knowledge. > > Actually I think that the first thing to decide is if memory usage by > tty should be bounded or not, and if yes, should it be per-tty limit, or > total memory usage by all the ttys limit, or both. Those decisions I'd > probably base on how other kernel subsystems behave (TCP stack is the > first that comes to mind, and AFAIK buffering for every socket is > limited). Due to lack of broad knowledge of the kernel, I won't try to > insist on any solution, even though my experience in embedded systems > programming cries for bounded model. > > And at the end, I'm going to RTFM ;) > > The comment to the throttle routine in the kernel tree says: > > * This routine notifies the tty driver that input buffers for > * the line discipline are close to full, and it should somehow > * signal that no more characters should be sent to the tty. > > "Linux Device Drivers" 3-d edition says: > > The throttle function is called when the tty core???s input buffers are > getting full. The tty driver should try to signal to the device that > no more characters should be sent to it. > > None of these two suggests there could be such a global consequences of > attempting to insert data to the throttled tty as exhausted kernel > memory. The kernel version reads more strict to me, but LDD one is > apparently how people indeed understand it. Well, as I wrote that chapter in LDD, that was how I understood it :) > BTW, I'm curious if Greg wasn't aware throttle must be handled, or just > decided that it's not worth to, as neither generic nor airprime > usb-serial drivers handle throttle. I wasn't aware that it was required. > Besides, the example tiny_tty.c driver from the LDD doesn't handle > throttle either. I wrote that too :) thanks, greg k-h ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
