Re: [Drbd-dev] [PATCH 068/118] drbd: conn_printk() a dev_printk() alike for drbd's connections

[Philipp Reisner]

Am Donnerstag, 25. August 2011, 20:16:25 schrieb Joe Perches:
> On Thu, 2011-08-25 at 17:08 +0200, Philipp Reisner wrote:
> > Signed-off-by: Philipp Reisner <philipp.reisner@xxxxxxxxxx>
> > Signed-off-by: Lars Ellenberg <lars.ellenberg@xxxxxxxxxx>
> > ---
> > 
> >  drivers/block/drbd/drbd_int.h  |    9 +++++++++
> >  drivers/block/drbd/drbd_main.c |   16 +++++++++++++++-
> >  2 files changed, 24 insertions(+), 1 deletions(-)
> 
> Couple of issues with this patch.
> 
> > diff --git a/drivers/block/drbd/drbd_int.h
> > b/drivers/block/drbd/drbd_int.h
> 
> []
> 
> > @@ -102,6 +102,15 @@ struct drbd_tconn;
> 
> []
> 
> > +extern void conn_printk(const char *level, struct drbd_tconn *tconn,
> > const char *fmt, ...);
> 
> This should be
> 
> extern __attribute__((format (printf, 3, 4)))
> void conn_printk(const char *level, struct drbd_tconn *tconn, const char 
> *fmt, ...);
> 
> to have gcc validate the printf format and arguments.
> 
> > diff --git a/drivers/block/drbd/drbd_main.c
> > b/drivers/block/drbd/drbd_main.c
> 
> []
> 
> > @@ -170,6 +170,18 @@ int _get_ldev_if_state(struct drbd_conf *mdev, enum
> > drbd_disk_state mins)
> > 
> >  #endif
> > 
> > +/* printk functions for connections
> > + */
> > +void conn_printk(const char *level, struct drbd_tconn *tconn, const char
> > *fmt, ...) +{
> > +	va_list args;
> > +
> > +	printk("%sd-con %s: ", level, tconn->name);
> > +	va_start(args, fmt);
> > +	vprintk(fmt, args);
> > +	va_end(args);
> 
> And using printk then vprintk is susceptible
> to another thread interleaving a different
> message between the printk and the vprintk.
> 
> Using struct va_format and %pV is better
> because no message interleaving is possible.
> 
[...]

Actually we came across the interleaving issue as
well and solved it at a later point in time by
converting it into a macro. I folded that into this
patch, which gives us: