linux-kernel - Re: [Drbd-dev] [PATCH 068/118] drbd: conn_printk() a dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <201108291342.08926.philipp.reisner@linbit.com>
Date:	Mon, 29 Aug 2011 13:42:08 +0200
From:	Philipp Reisner <philipp.reisner@...bit.com>
To:	Joe Perches <joe@...ches.com>, Jens Axboe <axboe@...nel.dk>,
	linux-kernel@...r.kernel.org
Cc:	drbd-dev@...ts.linbit.com
Subject: Re: [Drbd-dev] [PATCH 068/118] drbd: conn_printk() a dev_printk() alike for drbd's connections

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@...bit.com>
> > Signed-off-by: Lars Ellenberg <lars.ellenberg@...bit.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.
> 
[...]

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:


>From 3fd752956c255f85429f29603fd080e41ca2d90c Mon Sep 17 00:00:00 2001
From: Philipp Reisner <philipp.reisner@...bit.com>
Date: Mon, 7 Feb 2011 14:01:51 +0100
Subject: [PATCH 068/118] drbd: conn_printk() a dev_printk() alike for drbd's connections

Signed-off-by: Philipp Reisner <philipp.reisner@...bit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@...bit.com>
---
 drivers/block/drbd/drbd_int.h  |   10 ++++++++++
 drivers/block/drbd/drbd_main.c |    4 +++-
 2 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 927bf77..4d3320d 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -102,6 +102,16 @@ struct drbd_tconn;
 /* to shorten dev_warn(DEV, "msg"); and relatives statements */
 #define DEV (disk_to_dev(mdev->vdisk))
 
+#define conn_printk(LEVEL, TCONN, FMT, ARGS...) \
+       printk(LEVEL "d-con %s: " FMT, TCONN->name , ## ARGS)
+#define conn_alert(TCONN, FMT, ARGS...)  conn_printk(KERN_ALERT, TCONN, FMT, ## ARGS)
+#define conn_crit(TCONN, FMT, ARGS...)   conn_printk(KERN_CRIT, TCONN, FMT, ## ARGS)
+#define conn_err(TCONN, FMT, ARGS...)    conn_printk(KERN_ERR, TCONN, FMT, ## ARGS)
+#define conn_warn(TCONN, FMT, ARGS...)   conn_printk(KERN_WARNING, TCONN, FMT, ## ARGS)
+#define conn_notice(TCONN, FMT, ARGS...) conn_printk(KERN_NOTICE, TCONN, FMT, ## ARGS)
+#define conn_info(TCONN, FMT, ARGS...)   conn_printk(KERN_INFO, TCONN, FMT, ## ARGS)
+#define conn_dbg(TCONN, FMT, ARGS...)    conn_printk(KERN_DEBUG, TCONN, FMT, ## ARGS)
+
 #define D_ASSERT(exp)  if (!(exp)) \
         dev_err(DEV, "ASSERT( " #exp " ) in %s:%d\n", __FILE__, __LINE__)
 
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index dbd6d72..59a3166 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -2217,12 +2217,14 @@ struct drbd_conf *drbd_new_device(unsigned int minor)
        struct drbd_conf *mdev;
        struct gendisk *disk;
        struct request_queue *q;
+       char conn_name[9]; /* drbd1234N */
 
        /* GFP_KERNEL, we are outside of all write-out paths */
        mdev = kzalloc(sizeof(struct drbd_conf), GFP_KERNEL);
        if (!mdev)
                return NULL;
-       mdev->tconn = drbd_new_tconn("dummy");
+       sprintf(conn_name, "drbd%d", minor);
+       mdev->tconn = drbd_new_tconn(conn_name);
        if (!mdev->tconn)
                goto out_no_tconn;
 
-- 
1.7.4.1

-- 
: Dipl-Ing Philipp Reisner
: LINBIT | Your Way to High Availability
: Tel: +43-1-8178292-50, Fax: +43-1-8178292-82
: http://www.linbit.com

DRBD(R) and LINBIT(R) are registered trademarks of LINBIT, Austria.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/