From: Eran Liberty <liberty@extricom.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Subject: Re: [PATCH 2.6.26] SERIAL DRIVER: Handle Multiple consecutive sysrq from the serial
Date: Thu, 18 Sep 2008 09:58:36 +0300 [thread overview]
Message-ID: <48D1FC1C.90409@extricom.com> (raw)
In-Reply-To: <20080917164615.945ff068.akpm@linux-foundation.org>
Andrew Morton wrote:
> On Mon, 15 Sep 2008 19:30:05 +0300
> Eran Liberty <liberty@extricom.com> wrote:
>
>
>> Dear Penguins,
>>
>> Let me start of by saying my particular hardware must be buggy in some
>> way. When I issue a sysrq (Ctrl A+ F from minicom) I get a lot of sysrq
>> triggers.
>>
>> I have worked around the problem and I think this workaround is a viable
>> patch even for platforms which do not exhibit this peculiar behavior.
>>
>> upon getting numerous interrupts which request sysrq the function
>> uart_handle_break in include/linux/serial_core.h is hit multiple times.
>> The current code which looks like this:
>>
>> static inline int uart_handle_break(struct uart_port *port)
>> {
>> struct uart_info *info = port->info;
>> #ifdef SUPPORT_SYSRQ
>> if (port->cons && port->cons->index == port->line) {
>> if (!port->sysrq) {
>> port->sysrq = jiffies + HZ*5;
>> return 1;
>> }
>> port->sysrq = 0;
>> }
>> #endif
>> if (port->flags & UPF_SAK)
>> do_SAK(info->tty);
>> return 0;
>> }
>>
>> Will basicly toggle port->sysrq between a timeout value and zero. If you
>> are lucky this penguin rullet will stop on timeout and the next
>> character hit will trigger the sysrq in the function
>> "uart_handle_sysrq_char". But if you are not so lucky the last sysrq
>> interupt will toggle port->sysrq to zero and the next char hit will be
>> ignored (not trigger sysrq).
>>
>> The suggested patch will do the next few things:
>>
>> 1. "port->sysrq" is now the time when the last sysrq was triggered and
>> not the timeout for the the next char
>> 2. Stamped "port->sysrq" every time there is a sysrq rather then toggled
>> it up and down.
>> 3. Always continue to consider UPF_SAK.
>> 4. "port->sysrq" is toggled back to zero only in uart_handle_break() and
>> only if the a char has been accepted after the sysrq timeout (5 sec)
>> 5. uart_handle_break() will ignore extra chars received in super human
>> speed after the last sysrq (0.01 sec)
>>
>>
>
> yes, that could be irritating.
>
>
>> Index: include/linux/serial_core.h
>> ===================================================================
>> --- include/linux/serial_core.h (revision 119)
>> +++ include/linux/serial_core.h (revision 120)
>>
>
> We prefer patches in `patch -p1' form, please.
>
> Even after fixing that, none of it applied, so I typed it in again.
>
So should I resubmit or have you done then nasty work for me (thanks :) )?
>
>> @@ -447,8 +447,8 @@
>> uart_handle_sysrq_char(struct uart_port *port, unsigned int ch)
>> {
>> #ifdef SUPPORT_SYSRQ
>> - if (port->sysrq) {
>> - if (ch && time_before(jiffies, port->sysrq)) {
>> + if (port->sysrq && time_after(jiffies, port->sysrq + (unsigned long)(HZ*0.01))) {
>> + if (ch && time_before(jiffies, port->sysrq + HZ*5)) {
>> handle_sysrq(ch, port->info ? port->info->tty : NULL);
>> port->sysrq = 0;
>> return 1;
>> @@ -467,19 +467,17 @@
>> */
>> static inline int uart_handle_break(struct uart_port *port)
>> {
>> + int ret = 0;
>> struct uart_info *info = port->info;
>> #ifdef SUPPORT_SYSRQ
>> if (port->cons && port->cons->index == port->line) {
>> - if (!port->sysrq) {
>> - port->sysrq = jiffies + HZ*5;
>> - return 1;
>> - }
>> - port->sysrq = 0;
>> + port->sysrq = jiffies;
>> + ret = 1;
>> }
>> #endif
>> if (port->flags & UPF_SAK)
>> do_SAK(info->tty);
>> - return 0;
>> + return ret;
>> }
>>
>
> The 0.01 is a big no-no. Sometimes gcc like to go into stupid mode and
> starts doing floating point stuff.
>
> A suitable fix would be to use HZ/100. But that assumes that HZ is
> always >= 100. That's a pretty good assumption, and various parts of
> the kernel will explode if HZ is set too small. However it's always
> good to ensure that someone else's stuff will explode before yours
> does, so how about we make it HZ/50? Will that still work OK for you?
>
HZ/50 is totally OK by me.
-- Liberty
next prev parent reply other threads:[~2008-09-18 6:58 UTC|newest]
Thread overview: 81+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-18 14:18 2.6.24-rc4: pci_remove_bus_device() => pci_scan_child_bus() => pci_bus_add_devices bug? Eran Liberty
2008-07-20 10:31 ` [PATCH 2.6.24-rc4] PCI: refuse to re-add a device to a bus upon pci_scan_child_bus() Eran Liberty
2008-07-20 16:48 ` Eran Liberty
2008-07-21 19:18 ` [PATCH 2.6.26] " Eran Liberty
2008-07-21 19:49 ` Matthew Wilcox
2008-07-22 8:21 ` eran liberty
2008-07-22 11:49 ` Matthew Wilcox
2008-07-22 13:08 ` Eran Liberty
2008-07-22 13:14 ` Eran Liberty
2008-07-22 14:13 ` Matthew Wilcox
2008-07-22 15:25 ` Eran Liberty
2008-07-22 16:52 ` Matthew Wilcox
2008-07-22 17:41 ` Eran Liberty
2008-07-22 18:11 ` Matthew Wilcox
2008-07-23 18:31 ` Eran Liberty
2008-07-27 11:01 ` Eran Liberty
2008-07-27 15:08 ` Matthew Wilcox
2008-08-18 8:08 ` ftrace introduces instability into kernel 2.6.27(-rc2,-rc3) Eran Liberty
2008-08-18 15:07 ` Steven Rostedt
2008-08-18 15:47 ` Mathieu Desnoyers
2008-08-18 16:12 ` Steven Rostedt
2008-08-18 17:04 ` Mathieu Desnoyers
2008-08-18 17:21 ` Scott Wood
2008-08-18 18:27 ` Steven Rostedt
2008-08-18 18:29 ` Scott Wood
2008-08-19 1:53 ` Benjamin Herrenschmidt
2008-08-19 2:28 ` Steven Rostedt
2008-08-19 2:39 ` Benjamin Herrenschmidt
2008-08-19 2:41 ` Steven Rostedt
2008-08-19 2:47 ` Mathieu Desnoyers
2008-08-19 3:32 ` Steven Rostedt
2008-08-19 3:36 ` Mathieu Desnoyers
2008-08-19 4:00 ` Steven Rostedt
2008-08-19 16:47 ` Steven Rostedt
2008-08-19 17:34 ` Mathieu Desnoyers
2008-08-19 21:08 ` Steven Rostedt
2008-08-20 9:40 ` Nick Piggin
2008-08-19 21:47 ` Benjamin Herrenschmidt
2008-08-19 23:58 ` Jeremy Fitzhardinge
2008-08-20 1:17 ` Benjamin Herrenschmidt
2008-08-19 2:56 ` Benjamin Herrenschmidt
2008-08-19 3:12 ` Steven Rostedt
2008-08-19 4:17 ` Benjamin Herrenschmidt
2008-08-20 7:18 ` Benjamin Herrenschmidt
2008-08-20 13:14 ` Steven Rostedt
2008-08-20 13:19 ` Steven Rostedt
2008-08-20 13:36 ` Eran Liberty
2008-08-20 13:43 ` Steven Rostedt
2008-08-20 14:02 ` Eran Liberty
2008-08-20 14:55 ` Jon Smirl
2008-08-20 15:23 ` Steven Rostedt
2008-08-20 18:23 ` Eran Liberty
2008-08-20 18:33 ` Steven Rostedt
2008-08-20 15:27 ` Steven Rostedt
2008-08-20 21:37 ` Benjamin Herrenschmidt
2008-08-20 14:16 ` Josh Boyer
2008-08-20 14:22 ` Steven Rostedt
2008-08-20 14:50 ` Josh Boyer
2008-09-15 16:30 ` [PATCH 2.6.26] SERIAL DRIVER: Handle Multiple consecutive sysrq from the serial Eran Liberty
2008-09-17 23:46 ` Andrew Morton
2008-09-18 6:58 ` Eran Liberty [this message]
2008-08-20 21:36 ` ftrace introduces instability into kernel 2.6.27(-rc2,-rc3) Benjamin Herrenschmidt
2008-08-20 21:44 ` Steven Rostedt
2008-08-18 18:47 ` Steven Rostedt
2008-08-18 18:56 ` Scott Wood
2008-08-18 19:28 ` Steven Rostedt
2008-08-18 18:25 ` Eran Liberty
2008-08-18 18:41 ` Mathieu Desnoyers
2008-08-19 1:54 ` Benjamin Herrenschmidt
2008-08-19 9:56 ` Eran Liberty
2008-08-19 13:02 ` Mathieu Desnoyers
2008-08-19 21:46 ` Benjamin Herrenschmidt
2008-08-18 18:50 ` Steven Rostedt
2008-08-19 12:09 ` Eran Liberty
2008-08-19 13:05 ` Mathieu Desnoyers
2008-08-19 14:21 ` Eran Liberty
2008-08-19 14:42 ` Mathieu Desnoyers
2008-08-19 20:15 ` Steven Rostedt
2008-08-20 11:18 ` Eran Liberty
2008-08-20 13:12 ` Steven Rostedt
2008-08-19 1:51 ` Benjamin Herrenschmidt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=48D1FC1C.90409@extricom.com \
--to=liberty@extricom.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).