svn commit: r337518 - in head/sys: kern sys

[Kyle Evans]

On Thu, Aug 9, 2018 at 6:22 PM, Bruce Evans <brde at optusnet.com.au> wrote:
> On Thu, 9 Aug 2018, Kyle Evans wrote:
>
>> Log:
>>  kern: Add a BOOT_TAG marker at the beginning of boot dmesg
>>
>>  From the "newly licensed to drive" PR department, add a BOOT_TAG marker
>> (by
>>  default, --<<BOOT>>--, to the beginning of each boot's dmesg. This makes
>> it
>>  easier to do textproc magic to locate the start of each boot and, of
>>  particular interest to some, the dmesg of the current boot.
>>
>>  The PR has a dmesg(8) component as well that I've opted not to include
>> for
>>  the moment- it was the more contentious part of this PR.
>>
>>  bde@ also made the statement that this boot tag should be written with an
>>  ordinary printf, which I've- for the moment- declined to change about
>> this
>>  patch to keep it more transparent to observer of the boot process.
>
>
> Yes, it should be written with an ordinary printf.
>
>> Modified: head/sys/kern/subr_prf.c
>>
>> ==============================================================================
>> --- head/sys/kern/subr_prf.c    Thu Aug  9 01:17:35 2018        (r337517)
>> +++ head/sys/kern/subr_prf.c    Thu Aug  9 01:32:09 2018        (r337518)
>> @@ -1027,6 +1027,7 @@ msgbufinit(void *ptr, int size)
>>         cp = (char *)ptr;
>>         msgbufp = (struct msgbuf *)(cp + size);
>>         msgbuf_reinit(msgbufp, cp, size);
>> +       msgbuf_addstr(msgbufp, -1, BOOT_TAG, 0);
>>         if (msgbufmapped && oldp != msgbufp)
>>                 msgbuf_copy(oldp, msgbufp);
>>         msgbufmapped = 1;
>
>
> This seems to be wrong, since it writes the tag for all calls to
> msgbufinit(), giving multiple tags for multiple calls, but there should be
> only 1 per boot.  msgbufinit() should be called multiple times to bootstrap
> it.  My version does this.
>
> The only obvious bug in (a later version of) the output is a missing newline
> for the tag.
>
> My dmesg output on amd64:
> X ---<<BOOT>>------<<BOOT>>---Test of early printf to msgbuf
> X GDB: debug ports: sio
> X KDB: current port: sio
> X KDB: debugger backends: ddb gdb
> X KDB: current backend: ddb
> X Copyright (c) 1992-2018 The FreeBSD Project.
> X ...
>
> In -current, all of the lines before the copyright except the tag are broken
> by printf() of them to an unmapped msgbuf.  (The above output is without -v.
> Many more lines are lost with -v.)  In my version, msgbufinit() is called
> earlier with a small message buffer to hold the early output.

I've not seen a compelling reason to switch this to printf() rather
than direct addition to the msgbuf after it's initialized, but I'll
humor you on this one because I don't really care either way and
you've been at this a lot longer than I have.

When you say you call msgbufinit() earlier-- before the static
environment is even setup, or at least after that?

> Output across reboots also works right except for the missing newline:
>
> X ---<<BOOT>>------<<BOOT>>---Test of early printf to msgbuf
> X GDB: debug ports: sio
> X GDB: current port: sio
> X KDB: debugger backends: ddb gdb
> X KDB: current backend: ddb
> X Copyright (c) 1992-2018 The FreeBSD Project.
> X ...
> X Waiting (max 60 seconds) for system process `vnlru' to stop... done
> X Waiting (max 60 seconds) for system process `syncer' to stop... X Syncing
> disks, vnodes remaining... 0 0 0 done
> X Waiting (max 60 seconds) for system thread `bufdaemon' to stop... done
> X Waiting (max 60 seconds) for system thread `bufspacedaemon-0' to stop...
> done
> X Waiting (max 60 seconds) for system thread `bufspacedaemon-1' to stop...
> done
> X Waiting (max 60 seconds) for system thread `bufspacedaemon-2' to stop...
> done
> X Waiting (max 60 seconds) for system thread `bufspacedaemon-3' to stop...
> done
> X Waiting (max 60 seconds) for system thread `bufspacedaemon-4' to stop...
> done
> X Waiting (max 60 seconds) for system thread `bufspacedaemon-5' to stop...
> done
> X Waiting (max 60 seconds) for system thread `bufspacedaemon-6' to stop...
> done
> X All buffers synced.
> X ---<<BOOT>>------<<BOOT>>---Test of early printf to msgbuf
> X GDB: debug ports: sio
> X GDB: current port: sio
> X KDB: debugger backends: ddb gdb
> X KDB: current backend: ddb
> X Copyright (c) 1992-2018 The FreeBSD Project.
> X ...
>

Right- I tried to maintain the form of the original patch and also
overlooked this. My revised patch [1] prints the boot tag only upon
first init of the message buffer, swaps it over to printf, and adds a
newline to whatever's been chosen as the boot tag.

Thanks,

Kyle Evans

[1] https://people.freebsd.org/~kevans/bde-boottag.diff