Date: Thu, 9 Aug 2018 19:02:20 -0500 From: Kyle Evans <kevans@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r337518 - in head/sys: kern sys Message-ID: <CACNAnaF%2BEHEfv2%2BaqAkx-JDxaKWmS7uLefHLqjO7Te8h_30Mew@mail.gmail.com> In-Reply-To: <20180810081913.P1108@besplex.bde.org> References: <201808090132.w791WAQm057199@repo.freebsd.org> <20180810081913.P1108@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Aug 9, 2018 at 6:22 PM, Bruce Evans <brde@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACNAnaF%2BEHEfv2%2BaqAkx-JDxaKWmS7uLefHLqjO7Te8h_30Mew>