On 04/29/2015 07:02 PM, George Spelvin wrote:
>> So is trying to printk() a 1Kb log line over 9600 baud. That's nearly
>> a second with interrupts off; good luck with that.
>>
>> While on one hand, this seems like a flaw that we should be fixing,
>> on the the other hand, it is self-evident that serial console is
>> not the solution to every problem.
>>
>> I'm sure most kernel developers have experienced losing control of the
>> machine from console spew that never ends; which is why ftrace can be
>> so useful.
>
> I agree that it's impossible to fix 100%. Because console printk can happen
> from anywhere (specifically including a kmalloc(GFP_ATOMIC)), the input
> side must not take *any* locks, meaning it must not to *any* allocations;
> all memory it uses must be preallocated.
>
> And to guarantee no drops, you'd need enough space to accommodate the
> longest possible printk spew. Which, as you note, is not actually
> bounded. So preallocating enough space is impossible.
Exactly.
> (As we agreed last time, the baud rate is actually irrelevant;
> it's just the number of characters.)
Yeah; the reference above to 1Kb log line is simply that overall
machine stability can suffer in already-supported conditions (which is
only indirectly related to this specific problem).
Just pointing out that some uses of serial console have issues regardless.
> But it *is* possible to greatly ameliorate the situation. If the machine
> s *not* saturating the serial console, the few hundred bytes of buffers
> the serial port normally keeps around would handle a lot of debug
> messages. And the tty_buffers are already lockless.
I could see
1. preallocating buffers for the free list (although handling a preallocation
failure would either need to BUG or come from a pre-existing allocation
pool)
2. continually preallocating in the input worker if the free list is low
(requires accounting the free list)
3. disabling allocation from this input path (this will be ugly)
> Doing this is extremely possible. It's making the code clean enough
> that's the PITA that resulted in it getting pushed down my stack.
I see a couple of possibilities that might help:
1. Drop the CONFIG_xxxx knob for a build knob. For example, after the header
includes at the top of 8250_core.c:
#undef MY_BUILD_KNOB_OPTION
This means a patch or hand-edit is required to enable this option, like:
#define MY_BUILD_KNOB_OPTION 1
2. Instead of
#ifdef MY_BUILD_KNOB_OPTION
blah... blah...
#endif
use
if (IS_ENABLED(MY_BUILD_KNOB_OPTION)) {
blah... blah...
}
Same thing, but Greg will object less (I think).
3. Put the serial8250_rx_one_char() refactor in a separate, earlier patch.
4. The 'bits' parameter steering only needs to be build-enabled at the
outer-most caller; then, the wait_for_xmitr() hunk could be:
int rx = bits & UART_LSR_DR;
bits &= ~UART_LSR_DR;
5. Does the compiler complain if it can elide rx_while_wait_for_xmitr()?
Maybe that doesn't need to be conditional-compile?
Regards,
Peter Hurley
--
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
