From owner-freebsd-bugs@freebsd.org Fri Dec 18 18:53:37 2015 Return-Path: Delivered-To: freebsd-bugs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8E1B9A4C5EB for ; Fri, 18 Dec 2015 18:53:37 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by mx1.freebsd.org (Postfix) with ESMTP id 3CE791BF9 for ; Fri, 18 Dec 2015 18:53:36 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id A01E4D45D44; Sat, 19 Dec 2015 05:20:15 +1100 (AEDT) Date: Sat, 19 Dec 2015 05:20:14 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Eugene Grosbein cc: freebsd-bugs@freebsd.org Subject: Re: [Bug 205398] [regression] [tty] tty_drain() kernel function lacks timeout support it had before In-Reply-To: <56743674.2090104@grosbein.net> Message-ID: <20151219042026.Y1835@besplex.bde.org> References: <20151219013916.N930@besplex.bde.org> <56743674.2090104@grosbein.net> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=cK4dyQqN c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=nlC_4_pT8q9DhB4Ho9EA:9 a=KYN3fkho_wxR0NeMgegA:9 a=45ClL6m2LaAA:10 Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Dec 2015 18:53:37 -0000 On Fri, 18 Dec 2015, Eugene Grosbein wrote: > 18.12.2015 23:05, Bruce Evans =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> ... >> There is a hack for last-close that is supposed to give a hard-coded=20 >> timeout >> of 1 second. Not sure why this doesn't work for you. My quick fix that >> restores the timeout uses slightly different logic where this hack was. > > I've made a mistake (now corrected) while filling PR: my system is 9.3-ST= ABLE > and not 10.2-STABLE. It has no "leaving" case hack. I forgot to check that before. I first fixed this in FreeBSD-9, and that till doesn't have the hack. Later I merged my fix into FreeBSD-10 and reorganized it to be more similar to the surrounding code. The patch in my previous mail is relative to FreeBSD-10. Not much tty code has changed since then, and I don't use my complete tty patch set for -current (only parts for my drivers). > ... >> If the device is really disconnected, then the tty should be in a zombie >> state and should not wait. I think this still works. CLOCAL or lack of >> modem signals may break detection of last-close. > > The device does not get disconnected in process, it was not connected > from the moment of open(). > >> Did you have CRTSCTS flow control enabled? This is probably the main >> source of hangs. The RTS and CTS signals are not ignored in CLOCAL mode= , >> flow control should be invoked when they go away when th device goes >> away. > > It has both of CRTSCTS flow control and CLOCAL enabled and > I'd like to keep them both enabled and working. Reasonable, but I try to do the opposite for various reasons: - drivers should be able to keep up with low speeds like 115200 bps provided the CPU is an i386 at 20 MHz or faster. OS interrupt latency may break this. Data loss without RTSCTS serves as a warning about OS breakage. However, higher speeds are more of a problem (I test with 32 times faster and that has an inter-char time of 2.5 usec), and sometimes it is the external device that needs the flow control. - carrier detect (CD) is useful even for local connections. Sometimes you actually want unplugging a cable to cleanly terminate the connection. Bidirectional devices make CLOCAL mostly unnecessary. Note that they are mostly broken in -current. With non-broken ones, CLOCAL remains off throughout; CD is ignored for the purpose of opening, but acts later for the purpose of hanging up if it drops later (to drop, it must have been raised at some point either earlier or later). This works for most uses of modems. You open the callout device, usually before carrier is up, and program it. Then a connection raises carrier and dropping carrier should terminate the connection. Perhaps it shouldn't turn the device into a zombie, and the open fd should remain usable for further programming of the modem. But without CLOCAL, there is no way short of busy-waiting polling to get tell if the connection went away. The old version of the tty driver has some complications to track the TS_CONNECTED state independently of the TS_ZOMBIE state. I forget if this was needed for further programming of the modem. CLOCAL is now forced for callout devices, although this ignores the setting in the initial state device. This is to work around some other bugs. The correct workaround is the change the default for CLOCAL. >> This is mostly fixed in my version. I started to cut out the patches, >> but they were too entwined with other fixes. Here is the part that >> replaces the hard-coded 1 second timeout: >>=20 >> X diff -c2 ./kern/tty.c~ ./kern/tty.c >> X *** ./kern/tty.c~ Thu Mar 19 18:23:08 2015 >> X --- ./kern/tty.c Sat Aug 8 11:40:23 2015 >> X *************** >> X *** 133,155 **** >> X return (0); >> X X ! while (ttyoutq_bytesused(&tp->t_outq) > 0) { >> X ttydevsw_outwakeup(tp); >> X /* Could be handled synchronously. */ >> X bytesused =3D ttyoutq_bytesused(&tp->t_outq); >> X ! if (bytesused =3D=3D 0) >> X return (0); >> X X /* Wait for data to be drained. */ >> X ! if (leaving) { >> X revokecnt =3D tp->t_revokecnt; >> X ! error =3D tty_timedwait(tp, &tp->t_outwait, hz); >> X switch (error) { >> X case ERESTART: >> X if (revokecnt !=3D tp->t_revokecnt) >> X error =3D 0; >> X break; >> X case EWOULDBLOCK: >> X ! if (ttyoutq_bytesused(&tp->t_outq) < bytesused) >> X error =3D 0; >> X break; >> X } >> X --- 196,225 ---- >> X return (0); >> X X ! while (ttyoutq_bytesused(&tp->t_outq) !=3D 0 || tp->t_flags &= =20 >> TS_BUSY) { > > Strange diff format... Should patch(1) apply this with all those X'es ? The X is just quoting for mail, to try to prevent corruption of leading whitspace by mail programs. Unfortunately, so mail programs corrupt the X's instead, by joining them when one is on an empty line (the quoting is to add a leading "X " and that gives a trailing space for empty lines). patch(1) should indeed work with all those X's. Only the mailer corruption prevents it working. I didn't know this when I started using this quoting, and thought that the patch needed manual editing. I used to use a prefix of "% ". The prefix is chosen to be different from normal quoting, so that patch lines can be extracted using "grep '^X '" and then edited using sed (or use just sed if you know it well). Apparently patch was designed to work directly from quoted lines in mail format, or maybe shar format. I also like to interleave comments with the patch. The quoting and manual extraction is necessary for removing these lines. > Thank you for answer, anyway! I'll try to understand and test patches nex= t=20 > week. Here is a test program with interesting draining. It is mostly for debugging revoke(), but this required timing of the draining to get the revoke() to race with other activity. If the device is (mis)emulated or possibly if it has large buffers, or both, then timing test might fail, but it is probably easier to emulate the timing for large times than small ones. X /* XXX draining for smaller speeds/buffers is even more broken. */ X #define=09SPEED=099600 X #define=09ttys X #define=09uart X=20 X #include X #include X #include X #include X #include X=20 X #ifdef ttys X #ifdef uart X #define=09CODEV=09"/dev/cuau0" X #define=09DEV=09"/dev/cuau0" X #else X #define=09CODEV=09"/dev/cuad0" X #define=09DEV=09"/dev/cuad0" X #endif X #else X #define=09CODEV=09"/dev/ptyp0" X #define=09DEV=09"/dev/ttyp0" X #endif X=20 X #define CLOSEDRAINTIME=091 X #define LARGETIME=0910 X #define SMALLTIME=093 X=20 X char largebuf[SPEED / 10 * LARGETIME]; X char smallbuf[SPEED / 10 * SMALLTIME]; X=20 X int X main(void) X { X =09struct termios t; X =09int fd, fd0; X=20 X #ifdef CODEV X =09fd0 =3D open(CODEV, O_RDWR | O_NONBLOCK); X #endif X =09fd =3D open(DEV, O_RDWR | O_NONBLOCK); X =09tcgetattr(fd, &t); X =09cfsetspeed(&t, SPEED); X =09t.c_cflag &=3D ~CRTSCTS; X =09tcsetattr(fd, TCSADRAIN, &t); X =09fcntl(fd, F_SETFL, fcntl(fd, F_GETFL) & ~O_NONBLOCK); X =09memset(largebuf, 'a', sizeof(largebuf)); X =09memset(smallbuf, 'a', sizeof(smallbuf)); X =09if (fork() =3D=3D 0) { X =09=09write(fd, smallbuf, sizeof(smallbuf)); X =09=09printf( X =09=09"draining should take %d seconds at %d bps (but is broken)\n", X =09=09 sizeof(smallbuf) / (SPEED / 10), SPEED); X =09=09printf("drain smallbuf start\n"); X =09=09tcdrain(fd); X =09=09printf("drain smallbuf done\n"); X =09=09write(fd, largebuf, sizeof(largebuf)); X =09=09printf( X =09=09"draining should take %d seconds at %d bps (but is broken)\n", X =09=09 sizeof(largebuf) / (SPEED / 10), SPEED); X #ifdef hack X =09exit(0); X #endif X =09=09printf("drain largebuf start\n"); X =09=09tcdrain(fd); X =09=09close(fd); X =09=09printf("drain largebuf done\n"); X =09} else if (fork() =3D=3D 0) { X #ifdef testread X =09exit(0); # #endif X =09=09printf("read start\n"); X =09=09read(fd, smallbuf, sizeof(smallbuf)); X =09=09printf("read done\n"); X =09} else { X #ifdef hack X =09close(fd); X =09close(fd0); X #endif X =09=09usleep(1000000 * SMALLTIME | 1000000 * CLOSEDRAINTIME / 2); X =09=09printf("revoke\n"); X =09=09printf( X "revoke should flush, but actually drains, so should take a long time, bu= t\n"); X =09=09printf( X "it actually takes its hard-coded timeout of 1 second\n"); X =09=09revoke(DEV); X =09=09printf("revoke done\n"); X =09=09sleep(1); X =09=09printf("parent done\n"); X =09} X =09return (0); X } Bruce From owner-freebsd-bugs@freebsd.org Sat Dec 19 06:53:17 2015 Return-Path: Delivered-To: freebsd-bugs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3F618A4B943 for ; Sat, 19 Dec 2015 06:53:17 +0000 (UTC) (envelope-from bugzilla-noreply@freebsd.org) Received: from kenobi.freebsd.org (kenobi.freebsd.org [IPv6:2001:1900:2254:206a::16:76]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 306EC1C39 for ; Sat, 19 Dec 2015 06:53:17 +0000 (UTC) (envelope-from bugzilla-noreply@freebsd.org) Received: from bugs.freebsd.org ([127.0.1.118]) by kenobi.freebsd.org (8.15.2/8.15.2) with ESMTP id tBJ6rHvW023025 for ; Sat, 19 Dec 2015 06:53:17 GMT (envelope-from bugzilla-noreply@freebsd.org) From: bugzilla-noreply@freebsd.org To: freebsd-bugs@FreeBSD.org Subject: [Bug 205424] i386/boot: use macros from to represent eflags's bits Date: Sat, 19 Dec 2015 06:53:16 +0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: new X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: Base System X-Bugzilla-Component: misc X-Bugzilla-Version: 11.0-CURRENT X-Bugzilla-Keywords: X-Bugzilla-Severity: Affects Only Me X-Bugzilla-Who: kuleshovmail@gmail.com X-Bugzilla-Status: New X-Bugzilla-Priority: --- X-Bugzilla-Assigned-To: freebsd-bugs@FreeBSD.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: bug_id short_desc product version rep_platform op_sys bug_status bug_severity priority component assigned_to reporter cc attachments.created Message-ID: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Bugzilla-URL: https://bugs.freebsd.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 19 Dec 2015 06:53:17 -0000 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=205424 Bug ID: 205424 Summary: i386/boot: use macros from to represent eflags's bits Product: Base System Version: 11.0-CURRENT Hardware: Any OS: Any Status: New Severity: Affects Only Me Priority: --- Component: misc Assignee: freebsd-bugs@FreeBSD.org Reporter: kuleshovmail@gmail.com CC: avg@FreeBSD.org, jhb@FreeBSD.org Created attachment 164376 --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=164376&action=edit i386/boot: use macros from to represent eflags's bits Hello, Some i386/boot's source code contains some checks of the EFLAGS's bits. In the same time the header file provides macros which represents these bits. Let's use human readable names instead of numbers. Additionally indentation fixed and empty lines removed. -- You are receiving this mail because: You are the assignee for the bug.