Re: [patch 3/3] Example use of WARN()

Previous thread: sysbench+mysql(oltp, readonly) 30% regression with 2.6.26-rc1 by Zhang, Yanmin on Tuesday, May 6, 2008 - 9:55 pm. (27 messages)

Next thread: [PATCH] kbuild: escape meta characters in regular expression in make TAGS by Masatake YAMATO on Wednesday, May 7, 2008 - 12:04 am. (2 messages)
From: Arjan van de Ven
Date: Tuesday, May 6, 2008 - 11:20 pm

Subject: Rename WARN() to WARNING() to clear the namespace
From: Arjan van de Ven <arjan@linux.intel.com>

We want to use WARN() as a variant of WARN_ON(), however
a few drivers are using WARN() internally. This patch renames
these to WARNING() to avoid the namespace clash.
A few cases were defining but not using the thing, for those
cases I just deleted the definition.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/isdn/hisax/st5481.h       |    4 ++--
 drivers/isdn/hisax/st5481_b.c     |    4 ++--
 drivers/isdn/hisax/st5481_d.c     |    6 +++---
 drivers/isdn/hisax/st5481_usb.c   |   18 +++++++++---------
 drivers/usb/gadget/ether.c        |    2 --
 drivers/usb/gadget/file_storage.c |   14 +++++++-------
 drivers/usb/gadget/fsl_usb2_udc.c |    2 +-
 drivers/usb/gadget/fsl_usb2_udc.h |    2 +-
 drivers/usb/gadget/gmidi.c        |    2 --
 drivers/usb/gadget/goku_udc.c     |    2 +-
 drivers/usb/gadget/goku_udc.h     |    2 +-
 drivers/usb/gadget/inode.c        |    2 --
 drivers/usb/gadget/net2280.c      |    2 +-
 drivers/usb/gadget/net2280.h      |    2 +-
 drivers/usb/gadget/omap_udc.c     |    6 +++---
 drivers/usb/gadget/omap_udc.h     |    2 +-
 drivers/usb/gadget/pxa2xx_udc.c   |    4 ++--
 drivers/usb/gadget/pxa2xx_udc.h   |    2 +-
 drivers/usb/gadget/zero.c         |    2 --
 drivers/usb/host/isp116x-hcd.c    |    2 +-
 drivers/usb/host/isp116x.h        |    2 +-
 drivers/usb/host/sl811-hcd.c      |    2 +-
 drivers/usb/host/sl811.h          |    2 +-
 drivers/usb/misc/usbtest.c        |    4 ++--
 24 files changed, 42 insertions(+), 50 deletions(-)

diff --git a/drivers/isdn/hisax/st5481.h b/drivers/isdn/hisax/st5481.h
index 2044e71..cff7a63 100644
--- a/drivers/isdn/hisax/st5481.h
+++ b/drivers/isdn/hisax/st5481.h
@@ -220,7 +220,7 @@ enum {
 #define ERR(format, arg...) \
 printk(KERN_ERR "%s:%s: " format "\n" , __FILE__,  __func__ , ## arg)
 
-#define WARN(format, arg...) \
+#define WARNING(format, arg...) \
 ...
From: Arjan van de Ven
Date: Tuesday, May 6, 2008 - 11:21 pm

Subject: Add a WARN() macro; this is WARN_ON() + printk arguments
From: Arjan van de Ven <arjan@linux.intel.com>

Add a WARN() macro that acts like WARN_ON(), with the added feature that
it takes a printk like argument that is printed as part of the warning
message.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 include/asm-generic/bug.h |   16 ++++++++++++++++
 kernel/panic.c            |   24 ++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 2632328..dcefa5e 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -34,9 +34,13 @@ struct bug_entry {
 #ifndef __WARN
 #ifndef __ASSEMBLY__
 extern void warn_on_slowpath(const char *file, const int line);
+extern void warn_slowpath(const char *file, const int line, const char *fmt, ...);
 #define WANT_WARN_ON_SLOWPATH
 #endif
 #define __WARN() warn_on_slowpath(__FILE__, __LINE__)
+#define __WARN_printf(arg...) warn_slowpath(__FILE__, __LINE__, arg)
+#else
+#define __WARN_printf(arg...) __WARN()
 #endif
 
 #ifndef WARN_ON
@@ -48,6 +52,18 @@ extern void warn_on_slowpath(const char *file, const int line);
 })
 #endif
 
+#ifndef WARN
+#define WARN(condition, format...) ({						\
+	int __ret_warn_on = !!(condition);				\
+	if (unlikely(__ret_warn_on))					\
+		__WARN_printf(format);					\
+	unlikely(__ret_warn_on);					\
+})
+#endif
+
+
+
+
 #else /* !CONFIG_BUG */
 #ifndef HAVE_ARCH_BUG
 #define BUG()
diff --git a/kernel/panic.c b/kernel/panic.c
index 425567f..3e0c36b 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -318,6 +318,30 @@ void warn_on_slowpath(const char *file, int line)
 	add_taint(TAINT_WARN);
 }
 EXPORT_SYMBOL(warn_on_slowpath);
+
+
+void warn_slowpath(const char *file, int line, const char *fmt, ...)
+{
+	va_list args;
+
+
+	char function[KSYM_SYMBOL_LEN];
+	unsigned long caller = (unsigned long) __builtin_return_address(0);
+	sprint_symbol(function, ...
From: Arjan van de Ven
Date: Tuesday, May 6, 2008 - 11:21 pm

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Example use of WARN()

now that WARN() exists, we can fold some of the printk's into it

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

---
 lib/kobject.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index 718e510..960f4ed 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -164,9 +164,8 @@ static int kobject_add_internal(struct kobject *kobj)
 		return -ENOENT;
 
 	if (!kobj->name || !kobj->name[0]) {
-		pr_debug("kobject: (%p): attempted to be registered with empty "
+		WARN(1, "kobject: (%p): attempted to be registered with empty "
 			 "name!\n", kobj);
-		WARN_ON(1);
 		return -EINVAL;
 	}
 
@@ -576,12 +575,10 @@ static void kobject_release(struct kref *kref)
 void kobject_put(struct kobject *kobj)
 {
 	if (kobj) {
-		if (!kobj->state_initialized) {
-			printk(KERN_WARNING "kobject: '%s' (%p): is not "
+		if (!kobj->state_initialized)
+			WARN(1, KERN_WARNING "kobject: '%s' (%p): is not "
 			       "initialized, yet kobject_put() is being "
 			       "called.\n", kobject_name(kobj), kobj);
-			WARN_ON(1);
-		}
 		kref_put(&kobj->kref, kobject_release);
 	}
 }
-- 
1.5.4.5


--

From: Andrew Morton
Date: Tuesday, May 13, 2008 - 9:36 pm

Wanna take a look at mips allnoconfig, please?

lib/kobject.c: In function `kobject_add_internal':
lib/kobject.c:218: error: implicit declaration of function `WARN'

# CONFIG_BUG is not set

--

From: Arjan van de Ven
Date: Tuesday, May 13, 2008 - 9:51 pm

On Tue, 13 May 2008 21:36:06 -0700


I'll take a look tomorrow at work when I have access to more beefy
build stuff
--

From: Arjan van de Ven
Date: Wednesday, May 14, 2008 - 8:43 pm

On Tue, 13 May 2008 21:36:06 -0700

this should fix this


From: Arjan van de Ven <arjan@linux.intel.com>
Subject: [PATCH] Fix WARN() for the !CONFIG_BUG case

For the CONFIG_BUG=n case we need to provide a stub WARN() macro;
do this in the same style as WARN_ON() is done.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 include/asm-generic/bug.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index dcefa5e..1d2b51f 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -79,6 +79,14 @@ extern void warn_slowpath(const char *file, const int line, const char *fmt, ...
 	unlikely(__ret_warn_on);					\
 })
 #endif
+
+#ifndef WARN
+#define WARN(condition, format...) ({					\
+	int __ret_warn_on = !!(condition);				\
+	unlikely(__ret_warn_on);					\
+})
+#endif
+
 #endif
 
 #define WARN_ON_ONCE(condition)	({				\
-- 
1.5.4.5

--

From: Vegard Nossum
Date: Tuesday, May 6, 2008 - 11:41 pm

Hi!



Is there a good reason why this is not a static inline function? If
I've understood correctly, we want to turn as many macros as possible
into functions, and I don't see an immediate reason why this one can't

If WARN() is made a static inline, you can call
__builtin_return_address(0) there and pass it into here instead. This
seems like a kind of low-level internal function anyway, because of
the file/line info.

OTOH, why can't you use __FUNCTION__ or __func__ to determine the
caller (in WARN) rather than doing it here, at run-time? If it's to


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--

From: Arjan van de Ven
Date: Wednesday, May 7, 2008 - 6:37 am

On Wed, 7 May 2008 08:41:31 +0200

yes there is, and it's the same one that makes WARN_ON() not an inline
function (I tried); it's not possible to find a type for "condition"

if I could make it a static inline, I would make it an out of line
instead to save space ;)


--

From: Jiri Slaby
Date: Wednesday, May 7, 2008 - 12:31 am

__attribute__((format(printf,3,4)))
--

From: Johannes Weiner
Date: Wednesday, May 7, 2008 - 5:46 am

Hi Arjan,


It always bugged me that there is no WARN() that behaves similar to
BUG().

What do you think of making WARN() just emit the warning
unconditionally?

And maybe add the possibility to emit a text as well on both WARN() and
BUG()?  BUG("my kitten was hit by page %p!\n", page);

	Hannes
--

Previous thread: sysbench+mysql(oltp, readonly) 30% regression with 2.6.26-rc1 by Zhang, Yanmin on Tuesday, May 6, 2008 - 9:55 pm. (27 messages)

Next thread: [PATCH] kbuild: escape meta characters in regular expression in make TAGS by Masatake YAMATO on Wednesday, May 7, 2008 - 12:04 am. (2 messages)