All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] printk: Convert pr_<level> macros to functions
Date: Thu, 04 Mar 2010 13:29:52 -0800	[thread overview]
Message-ID: <1267738192.12993.48.camel@Joe-Laptop.home> (raw)
In-Reply-To: <20100304124943.ce7c1a63.akpm@linux-foundation.org>

On Thu, 2010-03-04 at 12:49 -0800, Andrew Morton wrote:
> On Wed, 03 Mar 2010 07:20:18 -0800 Joe Perches <joe@perches.com> wrote:
> > Maybe moving printed_len to file scope is racy somehow?
> Yes, printed_len will get corrupted when multiple CPU's are running
> printk/vprintk simultaneously.  That'll need to be fixed.

I think it'll be easier to use the %pV construct that
I posted today:

http://lkml.org/lkml/2010/3/4/19

> Is it possible to reduce the amount of
> code movement in the patch, make it a bit easier to follow?

I can do the move-around separately.

I think it'll be better as:

asmlinkage int pr_emerg(const char *fmt, ...)
{
	struct va_format vaf;
	va_list args;

	va_start(args, fmt);
	vaf.fmt = fmt;
	vaf.va = &args;
	r = printk(KERN_EMERG "%pV", &vaf);
	va_end(args);

	return r;
}
EXPORT_SYMBOL(pr_emerg);

I believe that avoids any racing.

> I think it would be justifiable to cook up a freaky macro and expand it
> eight times to avoid this duplication.  Ugly, but better than lots of
> duplication.

Maybe something like:

#define define_pr_level(function, level)	\
asmlinkage int function(const char *fmt, ...)	\
{						\
	struct va_format vaf;			\
	va_list args;				\
						\
	va_start(args, fmt);			\
	vaf.fmt = fmt;				\
	vaf.va = &args;				\
	r = printk(level "%pV", &vaf);		\
	va_end(args);				\
						\
	return r;				\
}						\
EXPORT_SYMBOL(function)

define_pr_level(pr_emerg,	KERN_EMERG);
define_pr_level(pr_alert,	KERN_ALERT);
define_pr_level(pr_crit,	KERN_CRIT);
define_pr_level(pr_err,		KERN_ERR);
define_pr_level(pr_warning,	KERN_WARNING);
define_pr_level(pr_notice,	KERN_NOTICE);
define_pr_level(pr_info,	KERN_INFO);

I think that's a bit ugly though myself.

> Or perhaps we can do it via a helper function which takes the
> additional argument?
> asmlinkage int pr_everything(char levelchar, const char *fmt, ...)

I believe that could not work.
The symbol names to link against wouldn't be found.



  reply	other threads:[~2010-03-04 21:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-03 15:20 [RFC PATCH] printk: Convert pr_<level> macros to functions Joe Perches
2010-03-04 20:49 ` Andrew Morton
2010-03-04 21:29   ` Joe Perches [this message]
2010-03-04 21:34     ` Andrew Morton
2010-03-04 21:40       ` Joe Perches
2010-03-04 22:00         ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1267738192.12993.48.camel@Joe-Laptop.home \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.