Messages in this thread | | | Subject | Re: [PATCH 068/118] drbd: conn_printk() a dev_printk() alike for drbd's connections | From | Joe Perches <> | Date | Thu, 25 Aug 2011 11:16:25 -0700 |
| |
On Thu, 2011-08-25 at 17:08 +0200, Philipp Reisner wrote: > Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com> > Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com> > --- > 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.
ie:
void conn_printk(const char *level, struct drbd_tconn *tconn, const char *fmt, ...) { struct va_format vaf; va_list args;
va_start(args, fmt);
vaf.fmt = fmt; vaf.va = &args;
printk("%sd-con %s: %pV", level, tconn->name, &vaf);
va_end(args); }
| |