[Toybox] [PATCH] fix dmesg default size/level filtering/performance

[Rob Landley]

On 11/24/14 21:50, enh wrote:
> On Mon, Nov 24, 2014 at 7:10 PM, Isaac Dunham <ibid.ag at gmail.com> wrote:
>> ...
>>   while ((cur = strchr(cur, '<') != 0) {
>>     memback(3, cur);
>>   }
>>
>>
>> This will call memmove() a lot; on the other hand, it can call write once.
> 
> yeah, the toolbox dmesg (which is "dmesg -r") only needs to do one big
> write, but for default current kernel buffers (128KiB) the difference
> between that and one syscall per line is not human-noticeable. judging
> by "strace -c dmesg" on the desktop, the competition doesn't bother
> either.

Yes, but as long as there _is_ a better way, I went ahead and did it the
toolbox way in the commit I just checked in. (Adjust the buffer
in-place, then xwrite() one big blob.)

I didn't bother with memmove() because a simple byte-assignment loop
doing a linear "read from one cache line, play with registers, write to
one cache line" loop that only fetches or flushes a new cache line every
few cycles and even that is as prefetch friendly as it gets. (Not just
fast: battery life, smp friendly, etc.) Given that the buffer size maxes
out at 2 megs in kconfig, I'm pretty happy calling that "good enough". :)

>> And I notice now that dmesg doesn't use the FLAG_* macros, but
>> instead hardcodes options by their offset in the optsring.
>> It was last touched about 2 years ago just after the FLAG_* macros,
>> so that isn't surprising.

That was my bad. I thought I'd grepped the code for all the old
"toys.optargs & constant" uses, apparently I missed some.

> cool. i'm assuming given how size-conscious you seem to be that i
> shouldn't add -r until someone actually asks for it though?

We're fairly size conscious, but dmesg.c dates from a time when we were
even more so. You'll notice I just added over a dozen lines to dmesg in
the name of making fewer system calls. (But we already have the buffer
and can filter in-place in a single pass, so one big write() makes sense.)

Toolbox had -r (or its behavior did). Adding it was ~2 lines. If toybox
dmesg is replacing toolbox dmesg in a nontrivial number of deployments,
it's just polite to add that. (Otherwise there's no way to get that
data, and it's data we already had that people may want...)

Rob

 1416975532.0