Andrew Morton wrote:
On Mon, 15 Sep 2008 19:30:05 +0300
Eran Liberty <liberty@xxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
