2010-11-10 00:35:28

by Joe Perches

[permalink] [raw]
Subject: [PATCH 0/9] treewide: convert vprintk uses to %pV

Multiple secessive calls to printk can be interleaved.
Avoid this possible interleaving by using %pV

Joe Perches (9):
drivers/gpu/drm/drm_stub.c: Use printf extension %pV
drivers/isdn/mISDN: Use printf extension %pV
drivers/net/wireless/ath/debug.c: Use printf extension %pV
drivers/net/wireless/b43/main.c: Use printf extension %pV
drivers/net/wireless/b43legacy/main.c: Use printf extension %pV
fs/gfs2/glock.c: Use printf extension %pV
fs/nilfs2/super.c: Use printf extension %pV
fs/quota/dquot.c: Use printf extension %pV
net/sunrpc/svc.c: Use printf extension %pV

drivers/gpu/drm/drm_stub.c | 14 +++++++--
drivers/isdn/mISDN/layer1.c | 10 +++++--
drivers/isdn/mISDN/layer2.c | 12 ++++++--
drivers/isdn/mISDN/tei.c | 23 +++++++++++----
drivers/net/wireless/ath/debug.c | 9 +++++-
drivers/net/wireless/b43/main.c | 48 ++++++++++++++++++++++++--------
drivers/net/wireless/b43legacy/main.c | 47 ++++++++++++++++++++++++--------
fs/gfs2/glock.c | 9 +++++-
fs/nilfs2/super.c | 23 +++++++++++-----
fs/quota/dquot.c | 12 +++++---
net/sunrpc/svc.c | 12 +++++---
11 files changed, 161 insertions(+), 58 deletions(-)

--
1.7.3.1.g432b3.dirty


2010-11-10 00:35:30

by Joe Perches

[permalink] [raw]
Subject: [PATCH 1/9] drivers/gpu/drm/drm_stub.c: Use printf extension %pV

Using %pV reduces the number of printk calls and
eliminates any possible message interleaving from
other printk calls.

Signed-off-by: Joe Perches <[email protected]>
---
drivers/gpu/drm/drm_stub.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index cdc89ee..e632527 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -57,13 +57,21 @@ void drm_ut_debug_printk(unsigned int request_level,
const char *function_name,
const char *format, ...)
{
+ struct va_format vaf;
va_list args;

if (drm_debug & request_level) {
- if (function_name)
- printk(KERN_DEBUG "[%s:%s], ", prefix, function_name);
va_start(args, format);
- vprintk(format, args);
+
+ vaf.fmt = format;
+ vaf.va = &args;
+
+ if (function_name)
+ printk(KERN_DEBUG "[%s:%s], %pV",
+ prefix, function_name, &vaf);
+ else
+ printk(KERN_DEBUG "%pV", &vaf);
+
va_end(args);
}
}
--
1.7.3.1.g432b3.dirty

2010-11-10 00:35:37

by Joe Perches

[permalink] [raw]
Subject: [PATCH 8/9] fs/quota/dquot.c: Use printf extension %pV

Using %pV reduces the number of printk calls and
eliminates any possible message interleaving from
other printk calls.

Signed-off-by: Joe Perches <[email protected]>
---
fs/quota/dquot.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 0fed41e..aad97ef 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -135,14 +135,18 @@ EXPORT_SYMBOL(dq_data_lock);
void __quota_error(struct super_block *sb, const char *func,
const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

if (printk_ratelimit()) {
va_start(args, fmt);
- printk(KERN_ERR "Quota error (device %s): %s: ",
- sb->s_id, func);
- vprintk(fmt, args);
- printk("\n");
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_ERR "Quota error (device %s): %s: %pV\n",
+ sb->s_id, func, &vaf);
+
va_end(args);
}
}
--
1.7.3.1.g432b3.dirty

2010-11-10 00:35:40

by Joe Perches

[permalink] [raw]
Subject: [PATCH 7/9] fs/nilfs2/super.c: Use printf extension %pV

Using %pV reduces the number of printk calls and
eliminates any possible message interleaving from
other printk calls.

Signed-off-by: Joe Perches <[email protected]>
---
fs/nilfs2/super.c | 23 ++++++++++++++++-------
1 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index f804d41..bef8cc6 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -111,12 +111,17 @@ void nilfs_error(struct super_block *sb, const char *function,
const char *fmt, ...)
{
struct nilfs_sb_info *sbi = NILFS_SB(sb);
+ struct va_format vaf;
va_list args;

va_start(args, fmt);
- printk(KERN_CRIT "NILFS error (device %s): %s: ", sb->s_id, function);
- vprintk(fmt, args);
- printk("\n");
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_CRIT "NILFS error (device %s): %s: %pV\n",
+ sb->s_id, function, &vaf);
+
va_end(args);

if (!(sb->s_flags & MS_RDONLY)) {
@@ -136,13 +141,17 @@ void nilfs_error(struct super_block *sb, const char *function,
void nilfs_warning(struct super_block *sb, const char *function,
const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

va_start(args, fmt);
- printk(KERN_WARNING "NILFS warning (device %s): %s: ",
- sb->s_id, function);
- vprintk(fmt, args);
- printk("\n");
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_WARNING "NILFS warning (device %s): %s: %pV\n",
+ sb->s_id, function, &vaf);
+
va_end(args);
}

--
1.7.3.1.g432b3.dirty

2010-11-10 00:35:43

by Joe Perches

[permalink] [raw]
Subject: [PATCH 9/9] net/sunrpc/svc.c: Use printf extension %pV

Using %pV reduces the number of printk calls and
eliminates any possible message interleaving from
other printk calls.

Signed-off-by: Joe Perches <[email protected]>
---
net/sunrpc/svc.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 6359c42..e28ddb3 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -962,6 +962,7 @@ static int
__attribute__ ((format (printf, 2, 3)))
svc_printk(struct svc_rqst *rqstp, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;
int r;
char buf[RPC_MAX_ADDRBUFLEN];
@@ -969,11 +970,14 @@ svc_printk(struct svc_rqst *rqstp, const char *fmt, ...)
if (!net_ratelimit())
return 0;

- printk(KERN_WARNING "svc: %s: ",
- svc_print_addr(rqstp, buf, sizeof(buf)));
-
va_start(args, fmt);
- r = vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ r = printk(KERN_WARNING "svc: %s: %pV",
+ svc_print_addr(rqstp, buf, sizeof(buf)), &vaf);
+
va_end(args);

return r;
--
1.7.3.1.g432b3.dirty

2010-11-10 00:35:33

by Joe Perches

[permalink] [raw]
Subject: [PATCH 2/9] drivers/isdn/mISDN: Use printf extension %pV

Using %pV reduces the number of printk calls and
eliminates any possible message interleaving from
other printk calls.

Signed-off-by: Joe Perches <[email protected]>
---
drivers/isdn/mISDN/layer1.c | 10 +++++++---
drivers/isdn/mISDN/layer2.c | 12 +++++++++---
drivers/isdn/mISDN/tei.c | 23 +++++++++++++++++------
3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/drivers/isdn/mISDN/layer1.c b/drivers/isdn/mISDN/layer1.c
index ac4aa18..5cc7c00 100644
--- a/drivers/isdn/mISDN/layer1.c
+++ b/drivers/isdn/mISDN/layer1.c
@@ -99,12 +99,16 @@ static void
l1m_debug(struct FsmInst *fi, char *fmt, ...)
{
struct layer1 *l1 = fi->userdata;
+ struct va_format vaf;
va_list va;

va_start(va, fmt);
- printk(KERN_DEBUG "%s: ", dev_name(&l1->dch->dev.dev));
- vprintk(fmt, va);
- printk("\n");
+
+ vaf.fmt = fmt;
+ vaf.va = &va;
+
+ printk(KERN_DEBUG "%s: %pV\n", dev_name(&l1->dch->dev.dev), &vaf);
+
va_end(va);
}

diff --git a/drivers/isdn/mISDN/layer2.c b/drivers/isdn/mISDN/layer2.c
index c973717..4ae7505 100644
--- a/drivers/isdn/mISDN/layer2.c
+++ b/drivers/isdn/mISDN/layer2.c
@@ -95,14 +95,20 @@ static void
l2m_debug(struct FsmInst *fi, char *fmt, ...)
{
struct layer2 *l2 = fi->userdata;
+ struct va_format vaf;
va_list va;

if (!(*debug & DEBUG_L2_FSM))
return;
+
va_start(va, fmt);
- printk(KERN_DEBUG "l2 (sapi %d tei %d): ", l2->sapi, l2->tei);
- vprintk(fmt, va);
- printk("\n");
+
+ vaf.fmt = fmt;
+ vaf.va = &va;
+
+ printk(KERN_DEBUG "l2 (sapi %d tei %d): %pV\n",
+ l2->sapi, l2->tei, &vaf);
+
va_end(va);
}

diff --git a/drivers/isdn/mISDN/tei.c b/drivers/isdn/mISDN/tei.c
index 1b85d9d..687c9b6 100644
--- a/drivers/isdn/mISDN/tei.c
+++ b/drivers/isdn/mISDN/tei.c
@@ -79,14 +79,19 @@ static void
da_debug(struct FsmInst *fi, char *fmt, ...)
{
struct manager *mgr = fi->userdata;
+ struct va_format vaf;
va_list va;

if (!(*debug & DEBUG_L2_TEIFSM))
return;
+
va_start(va, fmt);
- printk(KERN_DEBUG "mgr(%d): ", mgr->ch.st->dev->id);
- vprintk(fmt, va);
- printk("\n");
+
+ vaf.fmt = fmt;
+ vaf.va = &va;
+
+ printk(KERN_DEBUG "mgr(%d): %pV\n", mgr->ch.st->dev->id, &vaf);
+
va_end(va);
}

@@ -223,14 +228,20 @@ static void
tei_debug(struct FsmInst *fi, char *fmt, ...)
{
struct teimgr *tm = fi->userdata;
+ struct va_format vaf;
va_list va;

if (!(*debug & DEBUG_L2_TEIFSM))
return;
+
va_start(va, fmt);
- printk(KERN_DEBUG "sapi(%d) tei(%d): ", tm->l2->sapi, tm->l2->tei);
- vprintk(fmt, va);
- printk("\n");
+
+ vaf.fmt = fmt;
+ vaf.va = &va;
+
+ printk(KERN_DEBUG "sapi(%d) tei(%d): %pV\n",
+ tm->l2->sapi, tm->l2->tei, &vaf);
+
va_end(va);
}

--
1.7.3.1.g432b3.dirty

2010-11-10 00:36:31

by Joe Perches

[permalink] [raw]
Subject: [PATCH 5/9] drivers/net/wireless/b43legacy/main.c: Use printf extension %pV

Using %pV reduces the number of printk calls and
eliminates any possible message interleaving from
other printk calls.

Signed-off-by: Joe Perches <[email protected]>
---
drivers/net/wireless/b43legacy/main.c | 47 ++++++++++++++++++++++++--------
1 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/b43legacy/main.c b/drivers/net/wireless/b43legacy/main.c
index 67f18ec..1f11e16 100644
--- a/drivers/net/wireless/b43legacy/main.c
+++ b/drivers/net/wireless/b43legacy/main.c
@@ -181,52 +181,75 @@ static int b43legacy_ratelimit(struct b43legacy_wl *wl)

void b43legacyinfo(struct b43legacy_wl *wl, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

if (!b43legacy_ratelimit(wl))
return;
+
va_start(args, fmt);
- printk(KERN_INFO "b43legacy-%s: ",
- (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan");
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_INFO "b43legacy-%s: %pV",
+ (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan", &vaf);
+
va_end(args);
}

void b43legacyerr(struct b43legacy_wl *wl, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

if (!b43legacy_ratelimit(wl))
return;
+
va_start(args, fmt);
- printk(KERN_ERR "b43legacy-%s ERROR: ",
- (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan");
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_ERR "b43legacy-%s ERROR: %pV",
+ (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan", &vaf);
+
va_end(args);
}

void b43legacywarn(struct b43legacy_wl *wl, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

if (!b43legacy_ratelimit(wl))
return;
+
va_start(args, fmt);
- printk(KERN_WARNING "b43legacy-%s warning: ",
- (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan");
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_WARNING "b43legacy-%s warning: %pV",
+ (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan", &vaf);
+
va_end(args);
}

#if B43legacy_DEBUG
void b43legacydbg(struct b43legacy_wl *wl, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

va_start(args, fmt);
- printk(KERN_DEBUG "b43legacy-%s debug: ",
- (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan");
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_DEBUG "b43legacy-%s debug: %pV",
+ (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan", &vaf);
+
va_end(args);
}
#endif /* DEBUG */
--
1.7.3.1.g432b3.dirty

2010-11-10 00:36:30

by Joe Perches

[permalink] [raw]
Subject: [PATCH 6/9] fs/gfs2/glock.c: Use printf extension %pV

Using %pV reduces the number of printk calls and
eliminates any possible message interleaving from
other printk calls.

Signed-off-by: Joe Perches <[email protected]>
---
fs/gfs2/glock.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 8777885..d30b39c 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -952,17 +952,22 @@ int gfs2_glock_wait(struct gfs2_holder *gh)

void gfs2_print_dbg(struct seq_file *seq, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

va_start(args, fmt);
+
if (seq) {
struct gfs2_glock_iter *gi = seq->private;
vsprintf(gi->string, fmt, args);
seq_printf(seq, gi->string);
} else {
- printk(KERN_ERR " ");
- vprintk(fmt, args);
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_ERR " %pV", &vaf);
}
+
va_end(args);
}

--
1.7.3.1.g432b3.dirty

2010-11-10 00:37:00

by Joe Perches

[permalink] [raw]
Subject: [PATCH 4/9] drivers/net/wireless/b43/main.c: Use printf extension %pV

Using %pV reduces the number of printk calls and
eliminates any possible message interleaving from
other printk calls.

Signed-off-by: Joe Perches <[email protected]>
---
drivers/net/wireless/b43/main.c | 48 +++++++++++++++++++++++++++++---------
1 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index a118652..fa48803 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -322,59 +322,83 @@ static int b43_ratelimit(struct b43_wl *wl)

void b43info(struct b43_wl *wl, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

if (b43_modparam_verbose < B43_VERBOSITY_INFO)
return;
if (!b43_ratelimit(wl))
return;
+
va_start(args, fmt);
- printk(KERN_INFO "b43-%s: ",
- (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan");
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_INFO "b43-%s: %pV",
+ (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan", &vaf);
+
va_end(args);
}

void b43err(struct b43_wl *wl, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

if (b43_modparam_verbose < B43_VERBOSITY_ERROR)
return;
if (!b43_ratelimit(wl))
return;
+
va_start(args, fmt);
- printk(KERN_ERR "b43-%s ERROR: ",
- (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan");
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_ERR "b43-%s ERROR: %pV",
+ (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan", &vaf);
+
va_end(args);
}

void b43warn(struct b43_wl *wl, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

if (b43_modparam_verbose < B43_VERBOSITY_WARN)
return;
if (!b43_ratelimit(wl))
return;
+
va_start(args, fmt);
- printk(KERN_WARNING "b43-%s warning: ",
- (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan");
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_WARNING "b43-%s warning: %pV",
+ (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan", &vaf);
+
va_end(args);
}

void b43dbg(struct b43_wl *wl, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

if (b43_modparam_verbose < B43_VERBOSITY_DEBUG)
return;
+
va_start(args, fmt);
- printk(KERN_DEBUG "b43-%s debug: ",
- (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan");
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_DEBUG "b43-%s debug: %pV",
+ (wl && wl->hw) ? wiphy_name(wl->hw->wiphy) : "wlan", &vaf);
+
va_end(args);
}

--
1.7.3.1.g432b3.dirty

2010-11-10 00:37:23

by Joe Perches

[permalink] [raw]
Subject: [PATCH 3/9] drivers/net/wireless/ath/debug.c: Use printf extension %pV

Using %pV reduces the number of printk calls and
eliminates any possible message interleaving from
other printk calls.

Signed-off-by: Joe Perches <[email protected]>
---
drivers/net/wireless/ath/debug.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/debug.c b/drivers/net/wireless/ath/debug.c
index dacfb23..a9600ba 100644
--- a/drivers/net/wireless/ath/debug.c
+++ b/drivers/net/wireless/ath/debug.c
@@ -19,14 +19,19 @@

void ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;

if (likely(!(common->debug_mask & dbg_mask)))
return;

va_start(args, fmt);
- printk(KERN_DEBUG "ath: ");
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_DEBUG "ath: %pV", &vaf);
+
va_end(args);
}
EXPORT_SYMBOL(ath_print);
--
1.7.3.1.g432b3.dirty

2010-11-10 01:01:13

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 8/9] fs/quota/dquot.c: Use printf extension %pV

On Tue 09-11-10 16:35:22, Joe Perches wrote:
> Using %pV reduces the number of printk calls and
> eliminates any possible message interleaving from
> other printk calls.
Thanks. Merged.

Honza

>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> fs/quota/dquot.c | 12 ++++++++----
> 1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 0fed41e..aad97ef 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -135,14 +135,18 @@ EXPORT_SYMBOL(dq_data_lock);
> void __quota_error(struct super_block *sb, const char *func,
> const char *fmt, ...)
> {
> + struct va_format vaf;
> va_list args;
>
> if (printk_ratelimit()) {
> va_start(args, fmt);
> - printk(KERN_ERR "Quota error (device %s): %s: ",
> - sb->s_id, func);
> - vprintk(fmt, args);
> - printk("\n");
> +
> + vaf.fmt = fmt;
> + vaf.va = &args;
> +
> + printk(KERN_ERR "Quota error (device %s): %s: %pV\n",
> + sb->s_id, func, &vaf);
> +
> va_end(args);
> }
> }
> --
> 1.7.3.1.g432b3.dirty
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2010-11-10 02:16:35

by Kristian H. Kristensen

[permalink] [raw]
Subject: Re: [PATCH 1/9] drivers/gpu/drm/drm_stub.c: Use printf extension %pV

On Tue, Nov 9, 2010 at 7:35 PM, Joe Perches <[email protected]> wrote:
> Using %pV reduces the number of printk calls and
> eliminates any possible message interleaving from
> other printk calls.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
>  drivers/gpu/drm/drm_stub.c |   14 +++++++++++---
>  1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index cdc89ee..e632527 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -57,13 +57,21 @@ void drm_ut_debug_printk(unsigned int request_level,
>                         const char *function_name,
>                         const char *format, ...)
>  {
> +       struct va_format vaf;
>        va_list args;
>
>        if (drm_debug & request_level) {
> -               if (function_name)
> -                       printk(KERN_DEBUG "[%s:%s], ", prefix, function_name);
>                va_start(args, format);
> -               vprintk(format, args);
> +
> +               vaf.fmt = format;
> +               vaf.va = &args;
> +
> +               if (function_name)
> +                       printk(KERN_DEBUG "[%s:%s], %pV",
> +                              prefix, function_name, &vaf);
> +               else
> +                       printk(KERN_DEBUG "%pV", &vaf);

Wouldn't it be easier and more convenient to just make the %pV format
specifier just expect a format string and the va_arg list? Like this

printk(KERN_DEBUG "%pV", format, &args);

I mean, the %pV is kernel specific and we can just change how it works.

Kristian

2010-11-10 02:31:09

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/9] drivers/gpu/drm/drm_stub.c: Use printf extension %pV

On Tue, 2010-11-09 at 21:16 -0500, Kristian Høgsberg wrote:
> On Tue, Nov 9, 2010 at 7:35 PM, Joe Perches <[email protected]> wrote:
> > Using %pV reduces the number of printk calls and
> > eliminates any possible message interleaving from
> > other printk calls.

> Wouldn't it be easier and more convenient to just make the %pV format
> specifier just expect a format string and the va_arg list? Like this
> printk(KERN_DEBUG "%pV", format, &args);
> I mean, the %pV is kernel specific and we can just change how it works.

No it wouldn't.

gcc would now warn about a mismatch between format and arguments.


2010-11-10 20:55:32

by Steven Whitehouse

[permalink] [raw]
Subject: Re: [PATCH 6/9] fs/gfs2/glock.c: Use printf extension %pV

Hi,

Now in my -nmw GFS2 git tree along with the previous patch. Thanks,

Steve.

On Tue, 2010-11-09 at 16:35 -0800, Joe Perches wrote:
> Using %pV reduces the number of printk calls and
> eliminates any possible message interleaving from
> other printk calls.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> fs/gfs2/glock.c | 9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index 8777885..d30b39c 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -952,17 +952,22 @@ int gfs2_glock_wait(struct gfs2_holder *gh)
>
> void gfs2_print_dbg(struct seq_file *seq, const char *fmt, ...)
> {
> + struct va_format vaf;
> va_list args;
>
> va_start(args, fmt);
> +
> if (seq) {
> struct gfs2_glock_iter *gi = seq->private;
> vsprintf(gi->string, fmt, args);
> seq_printf(seq, gi->string);
> } else {
> - printk(KERN_ERR " ");
> - vprintk(fmt, args);
> + vaf.fmt = fmt;
> + vaf.va = &args;
> +
> + printk(KERN_ERR " %pV", &vaf);
> }
> +
> va_end(args);
> }
>

2010-11-10 21:19:09

by Joe Perches

[permalink] [raw]
Subject: [PATCH] fs/gfs2/glock.h: Add __attribute__((format(printf,2,3)) to gfs2_print_dbg

Functions that use printf formatting, especially
those that use %pV, should have their uses of
printf format and arguments checked by the compiler.

Signed-off-by: Joe Perches <[email protected]>
---
No current uses report any error in an allyesconfig build.

fs/gfs2/glock.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
index db1c26d..a12d117 100644
--- a/fs/gfs2/glock.h
+++ b/fs/gfs2/glock.h
@@ -212,6 +212,8 @@ int gfs2_glock_nq_num(struct gfs2_sbd *sdp,
int gfs2_glock_nq_m(unsigned int num_gh, struct gfs2_holder *ghs);
void gfs2_glock_dq_m(unsigned int num_gh, struct gfs2_holder *ghs);
void gfs2_glock_dq_uninit_m(unsigned int num_gh, struct gfs2_holder *ghs);
+
+__attribute__ ((format(printf, 2, 3)))
void gfs2_print_dbg(struct seq_file *seq, const char *fmt, ...);

/**

2010-11-10 21:35:03

by Steven Whitehouse

[permalink] [raw]
Subject: Re: [PATCH] fs/gfs2/glock.h: Add __attribute__((format(printf,2,3)) to gfs2_print_dbg

Hi,

Now in the GFS2 -nmw git tree. Thanks,

Steve.

On Wed, 2010-11-10 at 13:19 -0800, Joe Perches wrote:
> Functions that use printf formatting, especially
> those that use %pV, should have their uses of
> printf format and arguments checked by the compiler.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> No current uses report any error in an allyesconfig build.
>
> fs/gfs2/glock.h | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
> index db1c26d..a12d117 100644
> --- a/fs/gfs2/glock.h
> +++ b/fs/gfs2/glock.h
> @@ -212,6 +212,8 @@ int gfs2_glock_nq_num(struct gfs2_sbd *sdp,
> int gfs2_glock_nq_m(unsigned int num_gh, struct gfs2_holder *ghs);
> void gfs2_glock_dq_m(unsigned int num_gh, struct gfs2_holder *ghs);
> void gfs2_glock_dq_uninit_m(unsigned int num_gh, struct gfs2_holder *ghs);
> +
> +__attribute__ ((format(printf, 2, 3)))
> void gfs2_print_dbg(struct seq_file *seq, const char *fmt, ...);
>
> /**
>
>

2010-11-10 22:48:27

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 0/9] treewide: convert vprintk uses to %pV

On Tue, Nov 9, 2010 at 4:35 PM, Joe Perches <[email protected]> wrote:
> Multiple secessive calls to printk can be interleaved.
> Avoid this possible interleaving by using %pV
>
> Joe Perches (9):
>  drivers/gpu/drm/drm_stub.c: Use printf extension %pV
>  drivers/isdn/mISDN: Use printf extension %pV
>  drivers/net/wireless/ath/debug.c: Use printf extension %pV
>  drivers/net/wireless/b43/main.c: Use printf extension %pV
>  drivers/net/wireless/b43legacy/main.c: Use printf extension %pV
>  fs/gfs2/glock.c: Use printf extension %pV
>  fs/nilfs2/super.c: Use printf extension %pV
>  fs/quota/dquot.c: Use printf extension %pV
>  net/sunrpc/svc.c: Use printf extension %pV
>
>  drivers/gpu/drm/drm_stub.c            |   14 +++++++--
>  drivers/isdn/mISDN/layer1.c           |   10 +++++--
>  drivers/isdn/mISDN/layer2.c           |   12 ++++++--
>  drivers/isdn/mISDN/tei.c              |   23 +++++++++++----
>  drivers/net/wireless/ath/debug.c      |    9 +++++-
>  drivers/net/wireless/b43/main.c       |   48 ++++++++++++++++++++++++--------
>  drivers/net/wireless/b43legacy/main.c |   47 ++++++++++++++++++++++++--------
>  fs/gfs2/glock.c                       |    9 +++++-
>  fs/nilfs2/super.c                     |   23 +++++++++++-----
>  fs/quota/dquot.c                      |   12 +++++---
>  net/sunrpc/svc.c                      |   12 +++++---
>  11 files changed, 161 insertions(+), 58 deletions(-)

When was this added upstream BTW? I ask for backport considerations.

Luis

2010-11-10 23:01:26

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 0/9] treewide: convert vprintk uses to %pV

On Wed, 2010-11-10 at 14:48 -0800, Luis R. Rodriguez wrote:
> When was this added upstream BTW? I ask for backport considerations.

commit 7db6f5fb65a82af03229eef104dc9899c5eecf33
Author: Joe Perches <[email protected]>
Date: Sun Jun 27 01:02:33 2010 +0000

vsprintf: Recursive vsnprintf: Add "%pV", struct va_format

Add the ability to print a format and va_list from a structure pointer

Allows __dev_printk to be implemented as a single printk while
minimizing string space duplication.

%pV should not be used without some mechanism to verify the
format and argument use ala __attribute__(format (printf(...))).

Signed-off-by: Joe Perches <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: David S. Miller <[email protected]>

2010-11-11 02:38:27

by Ryusuke Konishi

[permalink] [raw]
Subject: Re: [PATCH 7/9] fs/nilfs2/super.c: Use printf extension %pV

On Tue, 9 Nov 2010 16:35:21 -0800, Joe Perches wrote:
> Using %pV reduces the number of printk calls and
> eliminates any possible message interleaving from
> other printk calls.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> fs/nilfs2/super.c | 23 ++++++++++++++++-------
> 1 files changed, 16 insertions(+), 7 deletions(-)

Applied to the nilfs tree.

Thank you.

Ryusuke Konishi

> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index f804d41..bef8cc6 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -111,12 +111,17 @@ void nilfs_error(struct super_block *sb, const char *function,
> const char *fmt, ...)
> {
> struct nilfs_sb_info *sbi = NILFS_SB(sb);
> + struct va_format vaf;
> va_list args;
>
> va_start(args, fmt);
> - printk(KERN_CRIT "NILFS error (device %s): %s: ", sb->s_id, function);
> - vprintk(fmt, args);
> - printk("\n");
> +
> + vaf.fmt = fmt;
> + vaf.va = &args;
> +
> + printk(KERN_CRIT "NILFS error (device %s): %s: %pV\n",
> + sb->s_id, function, &vaf);
> +
> va_end(args);
>
> if (!(sb->s_flags & MS_RDONLY)) {
> @@ -136,13 +141,17 @@ void nilfs_error(struct super_block *sb, const char *function,
> void nilfs_warning(struct super_block *sb, const char *function,
> const char *fmt, ...)
> {
> + struct va_format vaf;
> va_list args;
>
> va_start(args, fmt);
> - printk(KERN_WARNING "NILFS warning (device %s): %s: ",
> - sb->s_id, function);
> - vprintk(fmt, args);
> - printk("\n");
> +
> + vaf.fmt = fmt;
> + vaf.va = &args;
> +
> + printk(KERN_WARNING "NILFS warning (device %s): %s: %pV\n",
> + sb->s_id, function, &vaf);
> +
> va_end(args);
> }
>
> --
> 1.7.3.1.g432b3.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2010-11-16 18:22:50

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 2/9] drivers/isdn/mISDN: Use printf extension %pV

From: Joe Perches <[email protected]>
Date: Tue, 9 Nov 2010 16:35:16 -0800

> Using %pV reduces the number of printk calls and
> eliminates any possible message interleaving from
> other printk calls.
>
> Signed-off-by: Joe Perches <[email protected]>

Applied, thanks.