Date: Thu, 13 Oct 2011 14:40:53 -0400 From: Arnaud Lacombe <lacombar@gmail.com> To: FreeBSD Current <freebsd-current@freebsd.org> Cc: Arnaud Lacombe <lacombar@gmail.com> Subject: Re: [RFC] Prepend timestamp in msgbuf Message-ID: <CACqU3MWxn0rvbL5d7Vot35A%2BpfMDMEQdqmDgAaUABwzTxoH41Q@mail.gmail.com> In-Reply-To: <4e972734.52b1e00a.15ed.ffffd873@mx.google.com> References: <4e972734.52b1e00a.15ed.ffffd873@mx.google.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi, On Thu, Oct 13, 2011 at 2:00 PM, <lacombar@gmail.com> wrote: > From: Arnaud Lacombe <lacombar@gmail.com> > > Hi folks, > > There is many case recently when I really wished timestamp were present i= n the > post-mortem msgbuf. Such situation could be when userland application seg= fault > potentially triggering a panic/crash, or have information about the time-= wise > location of a given message (kernel or userland). > > Attached patch is available in the git repository at: > =A0git://github.com/lacombar/freebsd.git master/topic/msgbuf-timestamp > > Arnaud Lacombe (3): > =A0 =A0 =A0msgbuf(4): convert `msg_needsnl' to a bit flag > =A0 =A0 =A0msgbuf(4): add logic to prepend timestamp on new line > =A0 =A0 =A0msgbuf(4): add a sysctl to toggle timestamp prepend > > =A0sys/kern/subr_msgbuf.c | =A0 53 ++++++++++++++++++++++++++++++++++++++= ++------- > =A0sys/sys/msgbuf.h =A0 =A0 =A0 | =A0 =A04 ++- > =A02 files changed, 48 insertions(+), 9 deletions(-) > also tracked as: http://www.freebsd.org/cgi/query-pr.cgi?pr=3D161553 - Arnaud > diff --git a/sys/kern/subr_msgbuf.c b/sys/kern/subr_msgbuf.c > index cd9c551..b2f0e1a 100644 > --- a/sys/kern/subr_msgbuf.c > +++ b/sys/kern/subr_msgbuf.c > @@ -34,6 +34,7 @@ > =A0#include <sys/lock.h> > =A0#include <sys/mutex.h> > =A0#include <sys/msgbuf.h> > +#include <sys/sysctl.h> > > =A0/* > =A0* Maximum number conversion buffer length: uintmax_t in base 2, plus <= > > @@ -47,6 +48,13 @@ > =A0static u_int msgbuf_cksum(struct msgbuf *mbp); > > =A0/* > + * > + */ > +static int msgbuf_show_timestamp =3D 1; > +SYSCTL_INT(_kern, OID_AUTO, msgbuf_show_timestamp, CTLFLAG_RW, > + =A0 =A0&msgbuf_show_timestamp, 0, "Show timestamp in msgbuf"); > + > +/* > =A0* Initialize a message buffer of the specified size at the specified > =A0* location. This also zeros the buffer area. > =A0*/ > @@ -60,7 +68,7 @@ msgbuf_init(struct msgbuf *mbp, void *ptr, int size) > =A0 =A0 =A0 =A0msgbuf_clear(mbp); > =A0 =A0 =A0 =A0mbp->msg_magic =3D MSG_MAGIC; > =A0 =A0 =A0 =A0mbp->msg_lastpri =3D -1; > - =A0 =A0 =A0 mbp->msg_needsnl =3D 0; > + =A0 =A0 =A0 mbp->msg_flags =3D 0; > =A0 =A0 =A0 =A0bzero(&mbp->msg_lock, sizeof(mbp->msg_lock)); > =A0 =A0 =A0 =A0mtx_init(&mbp->msg_lock, "msgbuf", NULL, MTX_SPIN); > =A0} > @@ -95,7 +103,7 @@ msgbuf_reinit(struct msgbuf *mbp, void *ptr, int size) > > =A0 =A0 =A0 =A0mbp->msg_lastpri =3D -1; > =A0 =A0 =A0 =A0/* Assume that the old message buffer didn't end in a newl= ine. */ > - =A0 =A0 =A0 mbp->msg_needsnl =3D 1; > + =A0 =A0 =A0 mbp->msg_flags |=3D MSGBUF_NEEDNL; > =A0 =A0 =A0 =A0bzero(&mbp->msg_lock, sizeof(mbp->msg_lock)); > =A0 =A0 =A0 =A0mtx_init(&mbp->msg_lock, "msgbuf", NULL, MTX_SPIN); > =A0} > @@ -134,7 +142,7 @@ msgbuf_getcount(struct msgbuf *mbp) > =A0* The caller should hold the message buffer spinlock. > =A0*/ > =A0static inline void > -msgbuf_do_addchar(struct msgbuf *mbp, u_int *seq, int c) > +__msgbuf_do_addchar(struct msgbuf *mbp, u_int *seq, int c) > =A0{ > =A0 =A0 =A0 =A0u_int pos; > > @@ -149,6 +157,34 @@ msgbuf_do_addchar(struct msgbuf *mbp, u_int *seq, in= t c) > =A0 =A0 =A0 =A0*seq =3D MSGBUF_SEQNORM(mbp, *seq + 1); > =A0} > > +static inline void > +msgbuf_do_addchar(struct msgbuf *mbp, u_int *seq, int c) > +{ > + > + =A0 =A0 =A0 if (msgbuf_show_timestamp && mbp->msg_flags & MSGBUF_NEXT_N= EW_LINE) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 char buf[32], *bufp; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct timespec ts; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 int err; > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 buf[0] =3D '\0'; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 getnanouptime(&ts); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D snprintf(buf, sizeof buf, "[%d.%ld]= ", ts.tv_sec, ts.tv_nsec / 1000); > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bufp =3D buf; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 while (*bufp !=3D '\0') { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __msgbuf_do_addchar(mbp, se= q, *bufp); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bufp++; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mbp->msg_flags &=3D ~MSGBUF_NEXT_NEW_LINE; > + =A0 =A0 =A0 } > + > + =A0 =A0 =A0 __msgbuf_do_addchar(mbp, seq, c); > + > + =A0 =A0 =A0 if (c =3D=3D '\n') > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mbp->msg_flags |=3D MSGBUF_NEXT_NEW_LINE; > +} > + > =A0/* > =A0* Append a character to a message buffer. > =A0*/ > @@ -207,10 +243,10 @@ msgbuf_addstr(struct msgbuf *mbp, int pri, char *st= r, int filter_cr) > =A0 =A0 =A0 =A0 * did not end with a newline. =A0If that is the case, we = need to > =A0 =A0 =A0 =A0 * insert a newline before this string. > =A0 =A0 =A0 =A0 */ > - =A0 =A0 =A0 if (mbp->msg_lastpri !=3D pri && mbp->msg_needsnl !=3D 0) { > + =A0 =A0 =A0 if (mbp->msg_lastpri !=3D pri && (mbp->msg_flags & MSGBUF_N= EEDNL) !=3D 0) { > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0msgbuf_do_addchar(mbp, &seq, '\n'); > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 mbp->msg_needsnl =3D 0; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mbp->msg_flags &=3D ~MSGBUF_NEEDNL; > =A0 =A0 =A0 =A0} > > =A0 =A0 =A0 =A0for (i =3D 0; i < len; i++) { > @@ -219,7 +255,7 @@ msgbuf_addstr(struct msgbuf *mbp, int pri, char *str,= int filter_cr) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * (and therefore prefix_len !=3D 0), then= we need a priority > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * prefix for this line. > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */ > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (mbp->msg_needsnl =3D=3D 0 && prefix_len= !=3D 0) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((mbp->msg_flags & MSGBUF_NEEDNL) =3D=3D= 0 && prefix_len !=3D 0) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int j; > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0for (j =3D 0; j < prefix_l= en; j++) > @@ -242,9 +278,9 @@ msgbuf_addstr(struct msgbuf *mbp, int pri, char *str,= int filter_cr) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * we need to insert a new prefix or inser= t a newline later. > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (str[i] =3D=3D '\n') > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mbp->msg_needsnl =3D 0; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mbp->msg_flags &=3D ~MSGBUF= _NEEDNL; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mbp->msg_needsnl =3D 1; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mbp->msg_flags |=3D MSGBUF_= NEEDNL; > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0msgbuf_do_addchar(mbp, &seq, str[i]); > =A0 =A0 =A0 =A0} > @@ -395,3 +431,4 @@ msgbuf_copy(struct msgbuf *src, struct msgbuf *dst) > =A0 =A0 =A0 =A0while ((c =3D msgbuf_getchar(src)) >=3D 0) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0msgbuf_addchar(dst, c); > =A0} > + > diff --git a/sys/sys/msgbuf.h b/sys/sys/msgbuf.h > index 67f80a5..639ed72 100644 > --- a/sys/sys/msgbuf.h > +++ b/sys/sys/msgbuf.h > @@ -46,7 +46,9 @@ struct msgbuf { > =A0 =A0 =A0 =A0u_int =A0 =A0 =A0msg_cksum; =A0 =A0 =A0 =A0 =A0 /* checksu= m of contents */ > =A0 =A0 =A0 =A0u_int =A0 =A0 =A0msg_seqmod; =A0 =A0 =A0 =A0 =A0/* range f= or sequence numbers */ > =A0 =A0 =A0 =A0int =A0 =A0 =A0 =A0msg_lastpri; =A0 =A0 =A0 =A0 /* saved p= riority value */ > - =A0 =A0 =A0 int =A0 =A0 =A0 =A0msg_needsnl; =A0 =A0 =A0 =A0 /* set when= newline needed */ > + =A0 =A0 =A0 uint32_t =A0 msg_flags; > +#define MSGBUF_NEEDNL =A0 =A0 =A0 =A0 =A00x01 =A0 =A0/* set when newline= needed */ > +#define MSGBUF_NEXT_NEW_LINE =A0 0x02 > =A0 =A0 =A0 =A0struct mtx msg_lock; =A0 =A0 =A0 =A0 =A0 =A0/* mutex to pr= otect the buffer */ > =A0}; > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACqU3MWxn0rvbL5d7Vot35A%2BpfMDMEQdqmDgAaUABwzTxoH41Q>