2001-03-04 07:32:10

by Andrew Morton

[permalink] [raw]
Subject: [prepatches] removal of console_lock


All console-related activity curently happens under spin_lock_irqsave(&console_lock).
This causes interrutps to be blocked for 1-2 milliseconds with vgacon, and for
hundreds of milliseconds with fbdevs. This results in network overruns, audio
dropouts, dropped characters on serial ports and other such nice things.

This patch fixes it. Interrupts are enabled across all console operations.

It's still somewhat a work-in-progress.

- There are (pre-existing) races around the timer functions in
console.c. The dirty fix is to push all this stuff into keventd
context and to use acquire_cosole_sem(). The clean fix is to
rewrite the timer state machine.

Or do nothing. This thing only fires four times a day, the race
doesn't look like it'll crash the machine and there have been no
bug reports...

- console_print(). blah. What's this for? I just mapped it
onto printk(). My preferred option is to kill it off altogether.


I'm putting this out now for comments, and for interested parties to
test. Please. I'm serious.


Patches against 2.4.3-pre1 and 2.4.2-ac9 are at

http://www.uow.edu.au/~andrewm/linux/console.html

These patches are ~1600 lines and touch 35 files.



Some notes:

- show_console() had been removed. It's unused.

- console_tasklet has been replaced with a keventd callback,
`console_callback'.

- unblank_console() has gone. It was only used by panic(),
and...

- ... the call to poke_blanked_console() in vt_console_print() has
been restored. It doesn't deadlock during oopses now.

- arch/s390x/kernel/cpprintk.c has been removed.

- bust_spinlocks() has been enhanced (x86 and mips64).

- Major revamp of printk(). The approach taken in printk() is to try
to acquire the (new) console_sem. If we succeed, the output is
placed into the log buffer and is printed to the consoles. If we fail
to acquire the semaphore we just buffer the output in the log buffer
and the current holder of the console_sem will do the printing for us
prior to releasing console_sem.

If an oops is in progress we simply reset the locking and print
things immediately.

- Added a new syslog() mode: syslog(9, ...). It returns the number
of characters which are currently queued in the log buffer. Needed
to do this for kmsg_poll().

- Being heartily sick of reverse-engineering interface information
from implementations, I have gratuitously added comments in several
random parts of the kernel. Shoot me.

- Several video drivers are using spin_lock() in a timer
handler. There's a remote possibility that these can
deadlock (say, the timer fires again while the lock is held).
These have been changed to use _irqsave.

- The various low-level drivers have been reviewed to ensure that
they are safe when interrupts are enabled. Looks OK.

-


2001-03-04 10:28:38

by Pierre Rousselet

[permalink] [raw]
Subject: Re: [prepatches] removal of console_lock

Andrew Morton wrote:

> This patch fixes it. Interrupts are enabled across all console operations.
>
> It's still somewhat a work-in-progress.

The patch applies OK against 2.4.3-pre1
At the end of make bzImage I got
kerne/kernel.o(.text+0xcd00): undefined reference to 'in_interrupt'

PR
--
------------------------------------------------
Pierre Rousselet <[email protected]>
------------------------------------------------

2001-03-04 10:42:21

by Andrew Morton

[permalink] [raw]
Subject: Re: [prepatches] removal of console_lock

Pierre Rousselet wrote:
>
> Andrew Morton wrote:
>
> > This patch fixes it. Interrupts are enabled across all console operations.
> >
> > It's still somewhat a work-in-progress.
>
> The patch applies OK against 2.4.3-pre1
> At the end of make bzImage I got
> kerne/kernel.o(.text+0xcd00): undefined reference to 'in_interrupt'

Thanks. Add a

#include <linux/interrupt.h>

to kernel/pm.c

2001-03-04 10:58:14

by Manfred Spraul

[permalink] [raw]
Subject: Re: [prepatches] removal of console_lock

> - Major revamp of printk(). The approach taken in printk() is to try
> to acquire the (new) console_sem. If we succeed, the output is
> placed into the log buffer and is printed to the consoles. If we fail
> to acquire the semaphore we just buffer the output in the log buffer
> and the current holder of the console_sem will do the printing for us
> prior to releasing console_sem.

Is down_trylock reliable under load?

I remember 2 or 3 bug reports than disappeared after down_trylock was
removed.
The last one was this week.

http://www.mail-archive.com/[email protected]/msg31919.html

--
Manfred

2001-03-04 11:19:56

by Andrew Morton

[permalink] [raw]
Subject: Re: [prepatches] removal of console_lock

Manfred Spraul wrote:
>
> > - Major revamp of printk(). The approach taken in printk() is to try
> > to acquire the (new) console_sem. If we succeed, the output is
> > placed into the log buffer and is printed to the consoles. If we fail
> > to acquire the semaphore we just buffer the output in the log buffer
> > and the current holder of the console_sem will do the printing for us
> > prior to releasing console_sem.
>
> Is down_trylock reliable under load?

I'm not aware of any problems. It's only really used in one
place at present - when networking interrupt context wants to
acquire a socket's semaphore. Oh. That seems to have disappeared.

I've tested it pretty hard on SMP with zillions of printk()s from
interrupt context in parallel with `cat lots_of_stuff'.

> I remember 2 or 3 bug reports than disappeared after down_trylock was
> removed.
> The last one was this week.
>
> http://www.mail-archive.com/[email protected]/msg31919.html

That was different. The parport code was leaving the semaphore in
a downed state when it shouldn't have.

But you're right - it's not well-proven code. I'll go stare
angrily at it for a while. Of course, there are about ten
different implementations...

-

2001-03-05 06:29:11

by Cort Dougan

[permalink] [raw]
Subject: Re: [prepatches] removal of console_lock

I still get huge over-runs with fbdev (much improved, though).

Andrew, are you still working on it? If so, I'm happy to keep you
up-to-date on performance WRT Linux/PPC.

2001-03-05 13:02:21

by Andrew Morton

[permalink] [raw]
Subject: Re: [prepatches] removal of console_lock

Cort Dougan wrote:
>
> I still get huge over-runs with fbdev (much improved, though).

If you're referring to scheduling overruns then yes, you will
still see monstrous ones. We're still spending great lengths of
time in the kernel, only now we're doing it with interrupts
enabled. We can still block all tasks for half a second at a time.

This means that if you're telnetting into a machine and someone
cats a big file on the console, the system is completely unusable
until the `cat' completes.

(edit, edit, test, test)

OK, fixed.

> Andrew, are you still working on it? If so, I'm happy to keep you
> up-to-date on performance WRT Linux/PPC.

Please do.

I don't view this work as part of low-latency, BTW. Low
latency is a feature. The interrupt and scheduling
latencies caused by console+fbcon is a bug.


There are new patches at

http://www.uow.edu.au/~andrewm/linux/console.html

- Fixed a compile warning in i386_ksyms.c

- Include interrupt.h for UP builds

- Fixed a thinko which broke dmesg

- added console_conditional_schedule()

- used console_conditional_schedule() in four places.

This change allows the kernel to reschedule if needed while
performing lengthy console operations. Scheduling latency
is reduced from 500 milliseconds to 1 millisecond.

- Updated to latest kernels.


I think that's everything fixed. IWFM with Riva hardware. If
you still see huge latencies, please let me know how to
reproduce them - it's pretty trivial to fix it with the
new infrastructure.

BTW: the latest lolat patch still applies to 2.4.3-pre2.
People are after me for -ac patches as well, so I'll start
tracking Alan's kernels soon.

BTW2: testing methodology:

- Load netdriver with `debug=7'
- cat many files on VC1
- cat many files on VC2
- run netperf on VC3 to generate vast amounts of console and
log output in both process and interrupt context
- Madly flick between VCs


-

2001-03-05 14:37:34

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [prepatches] removal of console_lock

>Cort Dougan wrote:
>>
>> I still get huge over-runs with fbdev (much improved, though).
>
>If you're referring to scheduling overruns then yes, you will
>still see monstrous ones. We're still spending great lengths of
>time in the kernel, only now we're doing it with interrupts
>enabled. We can still block all tasks for half a second at a time.

Well, at least having interrupts enabled is really nice for us on PPC
since we have this nasty chip (the PMU) that don't like at all beeing
interrupted for a long time in the middle of a message exchange.
It has caused endless trouble in the past and still, occasionally, when
you set your powerbook display bit depth to 32 and don't enable HW
acceleration in the fbdev driver.

I hope your patch will be merged in as soon as possible :) I'll do some
tests here.

Ben.