[patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

[patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

[Arjan van de Ven]

Hi,

This patch series introduces WARN(), which is a WARN_ON() variant that
takes printk() like arguments. This was done after both akpm and I hit
several cases where this would have been useful; in addition, with
WARN(), we put the printk string inside the -----[ cut here ]-----
section, making it more likely that users (and tools like kerneloops)
pick up the message in addition to just the bare WARNING.

The first few patches have been in -mm for a long time; the later ones
are newer and introduce more users of WARN().

[patch 1/17] Clear the WARN() namespace

[Arjan van de Ven]

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>
Acked-by: Greg KH <greg@kroah.com>
Cc: Karsten Keil <kkeil@suse.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 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/at91_udc.h     |    2 
 drivers/usb/gadget/cdc2.c         |    2 
 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/printer.c      |    2 
 drivers/usb/gadget/pxa25x_udc.c   |    6 
 drivers/usb/gadget/pxa25x_udc.h   |    2 
 drivers/usb/gadget/u_ether.c      |    3 
 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 
 include/linux/usb/composite.h     |    2 
 usr/share/quilt/refresh           |  304 --------------------------------------
 29 files changed, 48 insertions(+), 359 deletions(-)

Index: linux.trees.git/drivers/isdn/hisax/st5481.h
===================================================================
--- linux.trees.git.orig/drivers/isdn/hisax/st5481.h
+++ linux.trees.git/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...) \
 printk(KERN_WARNING "%s:%s: " format "\n" , __FILE__,  __func__ , ## arg)
 
 #define INFO(format, arg...) \
@@ -412,7 +412,7 @@ struct st5481_adapter {
 ({ \
 	int status; \
 	if ((status = usb_submit_urb(urb, mem_flags)) < 0) { \
-		WARN("usb_submit_urb failed,status=%d", status); \
+		WARNING("usb_submit_urb failed,status=%d", status); \
 	} \
         status; \
 })
Index: linux.trees.git/drivers/isdn/hisax/st5481_b.c
===================================================================
--- linux.trees.git.orig/drivers/isdn/hisax/st5481_b.c
+++ linux.trees.git/drivers/isdn/hisax/st5481_b.c
@@ -180,7 +180,7 @@ static void usb_b_out_complete(struct ur
 				DBG(4,"urb killed status %d", urb->status);
 				return; // Give up
 			default: 
-				WARN("urb status %d",urb->status);
+				WARNING("urb status %d",urb->status);
 				if (b_out->busy == 0) {
 					st5481_usb_pipe_reset(adapter, (bcs->channel+1)*2 | USB_DIR_OUT, NULL, NULL);
 				}
@@ -372,6 +372,6 @@ void st5481_b_l2l1(struct hisax_if *ifc,
 		B_L1L2(bcs, PH_DEACTIVATE | INDICATION, NULL);
 		break;
 	default:
-		WARN("pr %#x\n", pr);
+		WARNING("pr %#x\n", pr);
 	}
 }
Index: linux.trees.git/drivers/isdn/hisax/st5481_d.c
===================================================================
--- linux.trees.git.orig/drivers/isdn/hisax/st5481_d.c
+++ linux.trees.git/drivers/isdn/hisax/st5481_d.c
@@ -389,7 +389,7 @@ static void usb_d_out_complete(struct ur
 				DBG(1,"urb killed status %d", urb->status);
 				break;
 			default: 
-				WARN("urb status %d",urb->status);
+				WARNING("urb status %d",urb->status);
 				if (d_out->busy == 0) {
 					st5481_usb_pipe_reset(adapter, EP_D_OUT | USB_DIR_OUT, fifo_reseted, adapter);
 				}
@@ -420,7 +420,7 @@ static void dout_start_xmit(struct FsmIn
 	isdnhdlc_out_init(&d_out->hdlc_state, 1, 0);
 
 	if (test_and_set_bit(buf_nr, &d_out->busy)) {
-		WARN("ep %d urb %d busy %#lx", EP_D_OUT, buf_nr, d_out->busy);
+		WARNING("ep %d urb %d busy %#lx", EP_D_OUT, buf_nr, d_out->busy);
 		return;
 	}
 	urb = d_out->urb[buf_nr];
@@ -601,7 +601,7 @@ void st5481_d_l2l1(struct hisax_if *hisa
 		FsmEvent(&adapter->d_out.fsm, EV_DOUT_START_XMIT, NULL);
 		break;
 	default:
-		WARN("pr %#x\n", pr);
+		WARNING("pr %#x\n", pr);
 		break;
 	}
 }
Index: linux.trees.git/drivers/isdn/hisax/st5481_usb.c
===================================================================
--- linux.trees.git.orig/drivers/isdn/hisax/st5481_usb.c
+++ linux.trees.git/drivers/isdn/hisax/st5481_usb.c
@@ -66,7 +66,7 @@ static void usb_ctrl_msg(struct st5481_a
 	struct ctrl_msg *ctrl_msg;
 	
 	if ((w_index = fifo_add(&ctrl->msg_fifo.f)) < 0) {
-		WARN("control msg FIFO full");
+		WARNING("control msg FIFO full");
 		return;
 	}
 	ctrl_msg = &ctrl->msg_fifo.data[w_index]; 
@@ -139,7 +139,7 @@ static void usb_ctrl_complete(struct urb
 				DBG(1,"urb killed status %d", urb->status);
 				return; // Give up
 			default: 
-				WARN("urb status %d",urb->status);
+				WARNING("urb status %d",urb->status);
 				break;
 		}
 	}
@@ -198,7 +198,7 @@ static void usb_int_complete(struct urb 
 			DBG(2, "urb shutting down with status: %d", urb->status);
 			return;
 		default:
-			WARN("nonzero urb status received: %d", urb->status);
+			WARNING("nonzero urb status received: %d", urb->status);
 			goto exit;
 	}
 
@@ -235,7 +235,7 @@ static void usb_int_complete(struct urb 
 exit:
 	status = usb_submit_urb (urb, GFP_ATOMIC);
 	if (status)
-		WARN("usb_submit_urb failed with result %d", status);
+		WARNING("usb_submit_urb failed with result %d", status);
 }
 
 /* ======================================================================
@@ -257,7 +257,7 @@ int st5481_setup_usb(struct st5481_adapt
 	DBG(2,"");
 	
 	if ((status = usb_reset_configuration (dev)) < 0) {
-		WARN("reset_configuration failed,status=%d",status);
+		WARNING("reset_configuration failed,status=%d",status);
 		return status;
 	}
 
@@ -269,7 +269,7 @@ int st5481_setup_usb(struct st5481_adapt
 
 	// Check if the config is sane
 	if ( altsetting->desc.bNumEndpoints != 7 ) {
-		WARN("expecting 7 got %d endpoints!", altsetting->desc.bNumEndpoints);
+		WARNING("expecting 7 got %d endpoints!", altsetting->desc.bNumEndpoints);
 		return -EINVAL;
 	}
 
@@ -279,7 +279,7 @@ int st5481_setup_usb(struct st5481_adapt
 
 	// Use alternative setting 3 on interface 0 to have 2B+D
 	if ((status = usb_set_interface (dev, 0, 3)) < 0) {
-		WARN("usb_set_interface failed,status=%d",status);
+		WARNING("usb_set_interface failed,status=%d",status);
 		return status;
 	}
 
@@ -497,7 +497,7 @@ static void usb_in_complete(struct urb *
 				DBG(1,"urb killed status %d", urb->status);
 				return; // Give up
 			default: 
-				WARN("urb status %d",urb->status);
+				WARNING("urb status %d",urb->status);
 				break;
 		}
 	}
@@ -523,7 +523,7 @@ static void usb_in_complete(struct urb *
 			DBG(4,"count=%d",status);
 			DBG_PACKET(0x400, in->rcvbuf, status);
 			if (!(skb = dev_alloc_skb(status))) {
-				WARN("receive out of memory\n");
+				WARNING("receive out of memory\n");
 				break;
 			}
 			memcpy(skb_put(skb, status), in->rcvbuf, status);
Index: linux.trees.git/drivers/usb/gadget/ether.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/ether.c
+++ linux.trees.git/drivers/usb/gadget/ether.c
@@ -293,7 +293,7 @@ static int __init eth_bind(struct usb_co
 		 * but if the controller isn't recognized at all then
 		 * that assumption is a bit more likely to be wrong.
 		 */
-		WARN(cdev, "controller '%s' not recognized; trying %s\n",
+		WARNING(cdev, "controller '%s' not recognized; trying %s\n",
 				gadget->name,
 				eth_config_driver.label);
 		device_desc.bcdDevice =
Index: linux.trees.git/drivers/usb/gadget/file_storage.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/file_storage.c
+++ linux.trees.git/drivers/usb/gadget/file_storage.c
@@ -308,7 +308,7 @@ MODULE_LICENSE("Dual BSD/GPL");
 	dev_vdbg(&(d)->gadget->dev , fmt , ## args)
 #define ERROR(d, fmt, args...) \
 	dev_err(&(d)->gadget->dev , fmt , ## args)
-#define WARN(d, fmt, args...) \
+#define WARNING(d, fmt, args...) \
 	dev_warn(&(d)->gadget->dev , fmt , ## args)
 #define INFO(d, fmt, args...) \
 	dev_info(&(d)->gadget->dev , fmt , ## args)
@@ -1091,7 +1091,7 @@ static int ep0_queue(struct fsg_dev *fsg
 	if (rc != 0 && rc != -ESHUTDOWN) {
 
 		/* We can't do much more than wait for a reset */
-		WARN(fsg, "error in submission: %s --> %d\n",
+		WARNING(fsg, "error in submission: %s --> %d\n",
 				fsg->ep0->name, rc);
 	}
 	return rc;
@@ -1227,7 +1227,7 @@ static void received_cbi_adsc(struct fsg
 
 	/* Save the command for later */
 	if (fsg->cbbuf_cmnd_size)
-		WARN(fsg, "CB[I] overwriting previous command\n");
+		WARNING(fsg, "CB[I] overwriting previous command\n");
 	fsg->cbbuf_cmnd_size = req->actual;
 	memcpy(fsg->cbbuf_cmnd, req->buf, fsg->cbbuf_cmnd_size);
 
@@ -1506,7 +1506,7 @@ static void start_transfer(struct fsg_de
 		 * submissions if DMA is enabled. */
 		if (rc != -ESHUTDOWN && !(rc == -EOPNOTSUPP &&
 						req->length == 0))
-			WARN(fsg, "error in submission: %s --> %d\n",
+			WARNING(fsg, "error in submission: %s --> %d\n",
 					ep->name, rc);
 	}
 }
@@ -2294,7 +2294,7 @@ static int halt_bulk_in_endpoint(struct 
 		VDBG(fsg, "delayed bulk-in endpoint halt\n");
 	while (rc != 0) {
 		if (rc != -EAGAIN) {
-			WARN(fsg, "usb_ep_set_halt -> %d\n", rc);
+			WARNING(fsg, "usb_ep_set_halt -> %d\n", rc);
 			rc = 0;
 			break;
 		}
@@ -2317,7 +2317,7 @@ static int wedge_bulk_in_endpoint(struct
 		VDBG(fsg, "delayed bulk-in endpoint wedge\n");
 	while (rc != 0) {
 		if (rc != -EAGAIN) {
-			WARN(fsg, "usb_ep_set_wedge -> %d\n", rc);
+			WARNING(fsg, "usb_ep_set_wedge -> %d\n", rc);
 			rc = 0;
 			break;
 		}
@@ -3755,7 +3755,7 @@ static int __init check_parameters(struc
 		if (gcnum >= 0)
 			mod_data.release = 0x0300 + gcnum;
 		else {
-			WARN(fsg, "controller '%s' not recognized\n",
+			WARNING(fsg, "controller '%s' not recognized\n",
 				fsg->gadget->name);
 			mod_data.release = 0x0399;
 		}
Index: linux.trees.git/drivers/usb/gadget/fsl_usb2_udc.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/fsl_usb2_udc.c
+++ linux.trees.git/drivers/usb/gadget/fsl_usb2_udc.c
@@ -1538,7 +1538,7 @@ static void dtd_complete_irq(struct fsl_
 
 		/* If the ep is configured */
 		if (curr_ep->name == NULL) {
-			WARN("Invalid EP?");
+			WARNING("Invalid EP?");
 			continue;
 		}
 
Index: linux.trees.git/drivers/usb/gadget/fsl_usb2_udc.h
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/fsl_usb2_udc.h
+++ linux.trees.git/drivers/usb/gadget/fsl_usb2_udc.h
@@ -552,7 +552,7 @@ static void dump_msg(const char *label, 
 #endif
 
 #define ERR(stuff...)		pr_err("udc: " stuff)
-#define WARN(stuff...)		pr_warning("udc: " stuff)
+#define WARNING(stuff...)		pr_warning("udc: " stuff)
 #define INFO(stuff...)		pr_info("udc: " stuff)
 
 /*-------------------------------------------------------------------------*/
Index: linux.trees.git/drivers/usb/gadget/gmidi.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/gmidi.c
+++ linux.trees.git/drivers/usb/gadget/gmidi.c
@@ -138,8 +138,6 @@ static void gmidi_transmit(struct gmidi_
 	dev_vdbg(&(d)->gadget->dev , fmt , ## args)
 #define ERROR(d, fmt, args...) \
 	dev_err(&(d)->gadget->dev , fmt , ## args)
-#define WARN(d, fmt, args...) \
-	dev_warn(&(d)->gadget->dev , fmt , ## args)
 #define INFO(d, fmt, args...) \
 	dev_info(&(d)->gadget->dev , fmt , ## args)
 
Index: linux.trees.git/drivers/usb/gadget/goku_udc.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/goku_udc.c
+++ linux.trees.git/drivers/usb/gadget/goku_udc.c
@@ -1768,7 +1768,7 @@ static int goku_probe(struct pci_dev *pd
 	 * usb_gadget_driver_{register,unregister}() must change.
 	 */
 	if (the_controller) {
-		WARN(dev, "ignoring %s\n", pci_name(pdev));
+		WARNING(dev, "ignoring %s\n", pci_name(pdev));
 		return -EBUSY;
 	}
 	if (!pdev->irq) {
Index: linux.trees.git/drivers/usb/gadget/goku_udc.h
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/goku_udc.h
+++ linux.trees.git/drivers/usb/gadget/goku_udc.h
@@ -285,7 +285,7 @@ struct goku_udc {
 
 #define ERROR(dev,fmt,args...) \
 	xprintk(dev , KERN_ERR , fmt , ## args)
-#define WARN(dev,fmt,args...) \
+#define WARNING(dev,fmt,args...) \
 	xprintk(dev , KERN_WARNING , fmt , ## args)
 #define INFO(dev,fmt,args...) \
 	xprintk(dev , KERN_INFO , fmt , ## args)
Index: linux.trees.git/drivers/usb/gadget/inode.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/inode.c
+++ linux.trees.git/drivers/usb/gadget/inode.c
@@ -262,8 +262,6 @@ static const char *CHIP;
 
 #define ERROR(dev,fmt,args...) \
 	xprintk(dev , KERN_ERR , fmt , ## args)
-#define WARN(dev,fmt,args...) \
-	xprintk(dev , KERN_WARNING , fmt , ## args)
 #define INFO(dev,fmt,args...) \
 	xprintk(dev , KERN_INFO , fmt , ## args)
 
Index: linux.trees.git/drivers/usb/gadget/net2280.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/net2280.c
+++ linux.trees.git/drivers/usb/gadget/net2280.c
@@ -1007,7 +1007,7 @@ static void scan_dma_completions (struct
 			 * 0122, and 0124; not all cases trigger the warning.
 			 */
 			if ((tmp & (1 << NAK_OUT_PACKETS)) == 0) {
-				WARN (ep->dev, "%s lost packet sync!\n",
+				WARNING (ep->dev, "%s lost packet sync!\n",
 						ep->ep.name);
 				req->req.status = -EOVERFLOW;
 			} else if ((tmp = readl (&ep->regs->ep_avail)) != 0) {
Index: linux.trees.git/drivers/usb/gadget/net2280.h
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/net2280.h
+++ linux.trees.git/drivers/usb/gadget/net2280.h
@@ -272,7 +272,7 @@ static inline void net2280_led_shutdown 
 
 #define ERROR(dev,fmt,args...) \
 	xprintk(dev , KERN_ERR , fmt , ## args)
-#define WARN(dev,fmt,args...) \
+#define WARNING(dev,fmt,args...) \
 	xprintk(dev , KERN_WARNING , fmt , ## args)
 #define INFO(dev,fmt,args...) \
 	xprintk(dev , KERN_INFO , fmt , ## args)
Index: linux.trees.git/drivers/usb/gadget/omap_udc.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/omap_udc.c
+++ linux.trees.git/drivers/usb/gadget/omap_udc.c
@@ -1120,7 +1120,7 @@ static int omap_ep_set_halt(struct usb_e
 			status = -EINVAL;
 		else if (value) {
 			if (ep->udc->ep0_set_config) {
-				WARN("error changing config?\n");
+				WARNING("error changing config?\n");
 				omap_writew(UDC_CLR_CFG, UDC_SYSCON2);
 			}
 			omap_writew(UDC_STALL_CMD, UDC_SYSCON2);
@@ -1764,7 +1764,7 @@ do_stall:
 					u.r.bRequestType, u.r.bRequest, status);
 			if (udc->ep0_set_config) {
 				if (udc->ep0_reset_config)
-					WARN("error resetting config?\n");
+					WARNING("error resetting config?\n");
 				else
 					omap_writew(UDC_CLR_CFG, UDC_SYSCON2);
 			}
@@ -3076,7 +3076,7 @@ static int omap_udc_suspend(struct platf
 	 * which would prevent entry to deep sleep...
 	 */
 	if ((devstat & UDC_ATT) != 0 && (devstat & UDC_SUS) == 0) {
-		WARN("session active; suspend requires disconnect\n");
+		WARNING("session active; suspend requires disconnect\n");
 		omap_pullup(&udc->gadget, 0);
 	}
 
Index: linux.trees.git/drivers/usb/gadget/omap_udc.h
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/omap_udc.h
+++ linux.trees.git/drivers/usb/gadget/omap_udc.h
@@ -188,7 +188,7 @@ struct omap_udc {
 #endif
 
 #define ERR(stuff...)		pr_err("udc: " stuff)
-#define WARN(stuff...)		pr_warning("udc: " stuff)
+#define WARNING(stuff...)	pr_warning("udc: " stuff)
 #define INFO(stuff...)		pr_info("udc: " stuff)
 #define DBG(stuff...)		pr_debug("udc: " stuff)
 
Index: linux.trees.git/drivers/usb/host/isp116x-hcd.c
===================================================================
--- linux.trees.git.orig/drivers/usb/host/isp116x-hcd.c
+++ linux.trees.git/drivers/usb/host/isp116x-hcd.c
@@ -882,7 +882,7 @@ static void isp116x_endpoint_disable(str
 	for (i = 0; i < 100 && !list_empty(&hep->urb_list); i++)
 		msleep(3);
 	if (!list_empty(&hep->urb_list))
-		WARN("ep %p not empty?\n", ep);
+		WARNING("ep %p not empty?\n", ep);
 
 	kfree(ep);
 	hep->hcpriv = NULL;
Index: linux.trees.git/drivers/usb/host/isp116x.h
===================================================================
--- linux.trees.git.orig/drivers/usb/host/isp116x.h
+++ linux.trees.git/drivers/usb/host/isp116x.h
@@ -338,7 +338,7 @@ struct isp116x_ep {
 #endif
 
 #define ERR(stuff...)		printk(KERN_ERR "116x: " stuff)
-#define WARN(stuff...)		printk(KERN_WARNING "116x: " stuff)
+#define WARNING(stuff...)	printk(KERN_WARNING "116x: " stuff)
 #define INFO(stuff...)		printk(KERN_INFO "116x: " stuff)
 
 /* ------------------------------------------------- */
Index: linux.trees.git/drivers/usb/host/sl811-hcd.c
===================================================================
--- linux.trees.git.orig/drivers/usb/host/sl811-hcd.c
+++ linux.trees.git/drivers/usb/host/sl811-hcd.c
@@ -1026,7 +1026,7 @@ sl811h_endpoint_disable(struct usb_hcd *
 	if (!list_empty(&hep->urb_list))
 		msleep(3);
 	if (!list_empty(&hep->urb_list))
-		WARN("ep %p not empty?\n", ep);
+		WARNING("ep %p not empty?\n", ep);
 
 	kfree(ep);
 	hep->hcpriv = NULL;
Index: linux.trees.git/drivers/usb/host/sl811.h
===================================================================
--- linux.trees.git.orig/drivers/usb/host/sl811.h
+++ linux.trees.git/drivers/usb/host/sl811.h
@@ -261,6 +261,6 @@ sl811_read_buf(struct sl811 *sl811, int 
 #endif
 
 #define ERR(stuff...)		printk(KERN_ERR "sl811: " stuff)
-#define WARN(stuff...)		printk(KERN_WARNING "sl811: " stuff)
+#define WARNING(stuff...)	printk(KERN_WARNING "sl811: " stuff)
 #define INFO(stuff...)		printk(KERN_INFO "sl811: " stuff)
 
Index: linux.trees.git/drivers/usb/misc/usbtest.c
===================================================================
--- linux.trees.git.orig/drivers/usb/misc/usbtest.c
+++ linux.trees.git/drivers/usb/misc/usbtest.c
@@ -81,7 +81,7 @@ static struct usb_device *testdev_to_usb
 
 #define ERROR(tdev, fmt, args...) \
 	dev_err(&(tdev)->intf->dev , fmt , ## args)
-#define WARN(tdev, fmt, args...) \
+#define WARNING(tdev, fmt, args...) \
 	dev_warn(&(tdev)->intf->dev , fmt , ## args)
 
 /*-------------------------------------------------------------------------*/
@@ -1946,7 +1946,7 @@ usbtest_probe (struct usb_interface *int
 
 			status = get_endpoints (dev, intf);
 			if (status < 0) {
-				WARN(dev, "couldn't get endpoints, %d\n",
+				WARNING(dev, "couldn't get endpoints, %d\n",
 						status);
 				return status;
 			}
Index: linux.trees.git/drivers/usb/gadget/printer.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/printer.c
+++ linux.trees.git/drivers/usb/gadget/printer.c
@@ -179,7 +179,7 @@ module_param(qlen, uint, S_IRUGO|S_IWUSR
 
 #define ERROR(dev, fmt, args...) \
 	xprintk(dev, KERN_ERR, fmt, ## args)
-#define WARN(dev, fmt, args...) \
+#define WARNING(dev, fmt, args...) \
 	xprintk(dev, KERN_WARNING, fmt, ## args)
 #define INFO(dev, fmt, args...) \
 	xprintk(dev, KERN_INFO, fmt, ## args)
Index: linux.trees.git/drivers/usb/gadget/pxa25x_udc.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/pxa25x_udc.c
+++ linux.trees.git/drivers/usb/gadget/pxa25x_udc.c
@@ -340,7 +340,7 @@ pxa25x_ep_free_request (struct usb_ep *_
 	struct pxa25x_request	*req;
 
 	req = container_of (_req, struct pxa25x_request, req);
-	WARN_ON (!list_empty (&req->queue));
+	WARN_ON(!list_empty (&req->queue));
 	kfree(req);
 }
 
@@ -1554,7 +1554,7 @@ config_change:
 					 * tell us about config change events,
 					 * so later ones may fail...
 					 */
-					WARN("config change %02x fail %d?\n",
+					WARNING("config change %02x fail %d?\n",
 						u.r.bRequest, i);
 					return;
 					/* TODO experiment:  if has_cfr,
@@ -2328,7 +2328,7 @@ static int pxa25x_udc_suspend(struct pla
 	unsigned long flags;
 
 	if (!udc->mach->gpio_pullup && !udc->mach->udc_command)
-		WARN("USB host won't detect disconnect!\n");
+		WARNING("USB host won't detect disconnect!\n");
 	udc->suspended = 1;
 
 	local_irq_save(flags);
Index: linux.trees.git/drivers/usb/gadget/cdc2.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/cdc2.c
+++ linux.trees.git/drivers/usb/gadget/cdc2.c
@@ -170,7 +170,7 @@ static int __init cdc_bind(struct usb_co
 		 * but if the controller isn't recognized at all then
 		 * that assumption is a bit more likely to be wrong.
 		 */
-		WARN(cdev, "controller '%s' not recognized; trying %s\n",
+		WARNING(cdev, "controller '%s' not recognized; trying %s\n",
 				gadget->name,
 				cdc_config_driver.label);
 		device_desc.bcdDevice =
Index: linux.trees.git/drivers/usb/gadget/u_ether.c
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/u_ether.c
+++ linux.trees.git/drivers/usb/gadget/u_ether.c
@@ -116,7 +116,6 @@ static inline int qlen(struct usb_gadget
 #undef DBG
 #undef VDBG
 #undef ERROR
-#undef WARN
 #undef INFO
 
 #define xprintk(d, level, fmt, args...) \
@@ -140,8 +139,6 @@ static inline int qlen(struct usb_gadget
 
 #define ERROR(dev, fmt, args...) \
 	xprintk(dev , KERN_ERR , fmt , ## args)
-#define WARN(dev, fmt, args...) \
-	xprintk(dev , KERN_WARNING , fmt , ## args)
 #define INFO(dev, fmt, args...) \
 	xprintk(dev , KERN_INFO , fmt , ## args)
 
Index: linux.trees.git/drivers/usb/gadget/at91_udc.h
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/at91_udc.h
+++ linux.trees.git/drivers/usb/gadget/at91_udc.h
@@ -171,7 +171,7 @@ struct at91_request {
 #endif
 
 #define ERR(stuff...)		pr_err("udc: " stuff)
-#define WARN(stuff...)		pr_warning("udc: " stuff)
+#define WARNING(stuff...)	pr_warning("udc: " stuff)
 #define INFO(stuff...)		pr_info("udc: " stuff)
 #define DBG(stuff...)		pr_debug("udc: " stuff)
 
Index: linux.trees.git/drivers/usb/gadget/pxa25x_udc.h
===================================================================
--- linux.trees.git.orig/drivers/usb/gadget/pxa25x_udc.h
+++ linux.trees.git/drivers/usb/gadget/pxa25x_udc.h
@@ -259,7 +259,7 @@ dump_state(struct pxa25x_udc *dev)
 #define DBG(lvl, stuff...) do{if ((lvl) <= UDC_DEBUG) DMSG(stuff);}while(0)
 
 #define ERR(stuff...)		pr_err("udc: " stuff)
-#define WARN(stuff...)		pr_warning("udc: " stuff)
+#define WARNING(stuff...)	pr_warning("udc: " stuff)
 #define INFO(stuff...)		pr_info("udc: " stuff)
 
 
Index: linux.trees.git/include/linux/usb/composite.h
===================================================================
--- linux.trees.git.orig/include/linux/usb/composite.h
+++ linux.trees.git/include/linux/usb/composite.h
@@ -330,7 +330,7 @@ extern int usb_string_id(struct usb_comp
 	dev_vdbg(&(d)->gadget->dev , fmt , ## args)
 #define ERROR(d, fmt, args...) \
 	dev_err(&(d)->gadget->dev , fmt , ## args)
-#define WARN(d, fmt, args...) \
+#define WARNING(d, fmt, args...) \
 	dev_warn(&(d)->gadget->dev , fmt , ## args)
 #define INFO(d, fmt, args...) \
 	dev_info(&(d)->gadget->dev , fmt , ## args)
Index: linux.trees.git/usr/share/quilt/refresh
===================================================================
--- linux.trees.git.orig/usr/share/quilt/refresh
+++ /dev/null
@@ -1,304 +0,0 @@
-#! /bin/bash
-
-#  This script is free software; you can redistribute it and/or modify
-#  it under the terms of the GNU General Public License version 2 as
-#  published by the Free Software Foundation.
-#
-#  See the COPYING and AUTHORS files for more details.
-
-# Read in library functions
-if [ "$(type -t patch_file_name)" != function ]
-then
-	if ! [ -r $QUILT_DIR/scripts/patchfns ]
-	then
-		echo "Cannot read library $QUILT_DIR/scripts/patchfns" >&2
-		exit 1
-	fi
-	. $QUILT_DIR/scripts/patchfns
-fi
-
-usage()
-{
-	printf $"Usage: quilt refresh [-p n|-p ab] [-u|-U num|-c|-C num] [-f] [--no-timestamps] [--no-index] [--diffstat] [--sort] [--backup] [--strip-trailing-whitespace] [patch]\n"
-
-	if [ x$1 = x-h ]
-	then
-		printf $"
-Refreshes the specified patch, or the topmost patch by default.
-Documentation that comes before the actual patch in the patch file is
-retained.
-
-It is possible to refresh patches that are not on top.  If any patches
-on top of the patch to refresh modify the same files, the script aborts
-by default.  Patches can still be refreshed with -f.  In that case this
-script will print a warning for each shadowed file, changes by more
-recent patches will be ignored, and only changes in files that have not
-been modified by any more recent patches will end up in the specified
-patch.
-
--p n	Create a -p n style patch (-p0 or -p1 supported).
-
--p ab	Create a -p1 style patch, but use a/file and b/file as the
-	original and new filenames instead of the default
-	dir.orig/file and dir/file names.
-
--u, -U num, -c, -C num
-	Create a unified diff (-u, -U) with num lines of context. Create
-	a context diff (-c, -C) with num lines of context. The number of
-	context lines defaults to 3.
-
---no-timestamps
-	Do not include file timestamps in patch headers.
-
---no-index
-	Do not output Index: lines.
-
---diffstat
-	Add a diffstat section to the patch header, or replace the
-	existing diffstat section.
-
--f	Enforce refreshing of a patch that is not on top.
-
---backup
-	Create a backup copy of the old version of a patch as patch~.
-
---sort	Sort files by their name instead of preserving the original order.
-
---strip-trailing-whitespace
-	Strip trailing whitespace at the end of lines.
-"
-		exit 0
-	else
-		exit 1
-	fi
-}
-
-die()
-{
-	local status=$1
-	[ -n "$tmp_patch" ] && rm -f $tmp_patch
-	[ -n "$tmp_header" ] && rm -f $tmp_header
-	[ -n "$tmp_result" ] && rm -f $tmp_result
-	exit $status
-}
-
-options=`getopt -o p:uU:cC:fh --long no-timestamps,diffstat,backup,sort \
-			      --long no-index \
-			      --long strip-trailing-whitespace -- "$@"`
-
-if [ $? -ne 0 ]
-then
-	usage
-fi
-
-eval set -- "$options"
-
-opt_format=-u
-while true
-do
-	case "$1" in
-	-p)
-		opt_strip_level=$2
-		shift 2 ;;
-	-f)
-		opt_force=1
-		shift ;;
-	-u|-c)
-		opt_format=$1
-		shift ;;
-	-U|-C)
-		opt_format="$1 $2"
-		shift 2 ;;
-	-h)
-		usage -h ;;
-	--no-timestamps)
-		QUILT_NO_DIFF_TIMESTAMPS=1
-		shift ;;
-	--no-index)
-		QUILT_NO_DIFF_INDEX=1
-		shift ;;
-	--diffstat)
-		opt_diffstat=1
-		shift ;;
-	--backup)
-		QUILT_BACKUP=1
-		shift ;;
-	--sort)
-		opt_sort=1
-		shift ;;
-	--strip-trailing-whitespace)
-		opt_strip_whitespace=1
-		shift ;;
-	--)
-		shift
-		break ;;
-	esac
-done
-
-if [ $# -gt 1 ]
-then
-	usage
-fi
-
-QUILT_DIFF_OPTS="$QUILT_DIFF_OPTS $opt_format"
-
-patch=$(find_applied_patch "$1") || exit 1
-
-if [ -z "$opt_strip_level" ]
-then
-	opt_strip_level=$(patch_strip_level $patch)
-fi
-case "$opt_strip_level" in
-0 | 1)
-	num_strip_level=$opt_strip_level
-	;;
-ab)
-	num_strip_level=1
-	;;
-*)
-	printf $"Cannot refresh patches with -p%s, please specify -p0 or -p1 instead\n" "$opt_strip_level\n" >&2
-	exit 1
-	;;
-esac
-
-trap "die 1" SIGTERM
-
-tmp_patch=$(gen_tempfile)
-
-if [ -z "$opt_sort" ]
-then
-	files=( $(files_in_patch_ordered $patch) )
-else
-	files=( $(files_in_patch $patch | sort) )
-fi
-
-for file in "${files[@]}"
-do
-	old_file=$(backup_file_name $patch $file)
-	next_patch=$(next_patch_for_file $patch $file)
-	if [ -z "$next_patch" ]
-	then
-		new_file=$file
-	else
-		new_file=$(backup_file_name $next_patch $file)
-		files_were_shadowed=1
-	fi
-	if ! diff_file $file $old_file $new_file >> $tmp_patch
-	then
-		printf $"Diff failed, aborting\n" >&2
-		die 1
-	fi
-
-	if [ -n "$files_were_shadowed" -a -z "$opt_force" ]
-	then
-		printf $"More recent patches modify files in patch %s. Enforce refresh with -f.\n" "$(print_patch $patch)" >&2
-		die 1
-	fi
-
-	if [ -n "$files_were_shadowed" -a -n "$opt_strip_whitespace" ]
-	then
-		printf $"Cannot use --strip-trailing-whitespace on a patch that has shadowed files.\n" >&2
-	fi
-done
-
-if ! [ -s $tmp_patch ]
-then
-	printf $"Nothing in patch %s\n" "$(print_patch $patch)" >&2
-	die 1
-fi
-
-# Check for trailing whitespace
-if [ -z "$opt_strip_whitespace" ]
-then
-	$QUILT_DIR/scripts/remove-trailing-ws -n -p$num_strip_level \
-	< $tmp_patch
-else
-	tmp_patch2=$(gen_tempfile)
-	if $QUILT_DIR/scripts/remove-trailing-ws -p$num_strip_level \
-				       < $tmp_patch > $tmp_patch2
-	then
-		rm -f $tmp_patch
-		tmp_patch=$tmp_patch2
-	fi
-fi
-# FIXME: no stripping of non-topmost patch !!!
-
-patch_file=$(patch_file_name $patch)
-
-trap "" SIGINT
-
-if ! tmp_header=$(gen_tempfile)
-then
-	die 1
-fi
-
-mkdir -p $(dirname $patch_file)
-
-if ! cat_file $patch_file | patch_header > $tmp_header
-then
-	die 1
-fi
-
-tmp_result=$(gen_tempfile) || die 1
-
-if [ -n "$opt_diffstat" ]
-then
-	diffstat="$(diffstat $QUILT_DIFFSTAT_OPTS \
-			     -p$num_strip_level $tmp_patch)" || die 1
-	awk '
-	    function print_diffstat(arr, i) {
-	      split(diffstat, arr, "\n")
-	      for (i=1; i in arr; i++)
-		print prefix arr[i]
-	    }
-			{ prefix=""
-			  if (index($0, "#") == 1)
-			    prefix="#"
-			}
-	    /^#? .* \|  *[1-9][0-9]* /	{ eat = eat $0 "\n"
-			  next }
-	    /^#? .* files? changed(, .* insertions?\(\+\))?(, .* deletions?\(-\))?/ \
-			{ print_diffstat()
-			  diffstat = "" ; eat = ""
-			  next }
-			{ print eat $0
-			  eat = "" }
-	    END		{ printf "%s", eat
-	    		  if (diffstat != "") {
-			    print "---"
-			    print_diffstat()
-			    print prefix
-			  }
-			}
-	    ' diffstat="$diffstat" \
-	    $tmp_header > $tmp_result
-else
-	cat $tmp_header > $tmp_result
-fi
-
-cat $tmp_patch >> $tmp_result
-
-if [ -e $patch_file ] && \
-   diff -q $patch_file $tmp_result > /dev/null
-then
-	printf $"Patch %s is unchanged\n" "$(print_patch $patch)"
-elif ( [ -z "$QUILT_BACKUP" -o ! -e $patch_file ] || \
-       mv $patch_file $patch_file~ ) && \
-     cat_to_new_file $patch_file < $tmp_result
-then
-	touch $QUILT_PC/$patch/.timestamp
-	printf $"Refreshed patch %s\n" "$(print_patch $patch)"
-else
-	die 1
-fi
-
-rm -f $QUILT_PC/$patch~refresh
-if ! change_db_strip_level -p$num_strip_level $patch
-then
-	die 1
-fi
-die 0
-### Local Variables:
-### mode: shell-script
-### End:
-# vim:filetype=sh

Re: [patch 1/17] Clear the WARN() namespace

[Andrew Morton]

On Tue, 8 Jul 2008 09:39:30 -0700 Arjan van de Ven <arjan@infradead.org> wrote:

> 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>
> Acked-by: Greg KH <greg@kroah.com>
> Cc: Karsten Keil <kkeil@suse.de>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  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/at91_udc.h     |    2 
>  drivers/usb/gadget/cdc2.c         |    2 
>  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/printer.c      |    2 
>  drivers/usb/gadget/pxa25x_udc.c   |    6 
>  drivers/usb/gadget/pxa25x_udc.h   |    2 
>  drivers/usb/gadget/u_ether.c      |    3 
>  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 
>  include/linux/usb/composite.h     |    2 

After fixing rejects, this appears to be identical to what I already had.


>  usr/share/quilt/refresh           |  304 --------------------------------------

Except for this curosity, which I rather hope wasn't supposed to be there.

[patch 2/17] Add a WARN() macro that acts like WARN_ON()+printk

[Arjan van de Ven]

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>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/asm-generic/bug.h |   32 ++++++++++++++++++++++++++++++++
 kernel/panic.c            |   22 ++++++++++++++++++++++
 2 files changed, 54 insertions(+)

Index: linux.trees.git/include/asm-generic/bug.h
===================================================================
--- linux.trees.git.orig/include/asm-generic/bug.h
+++ linux.trees.git/include/asm-generic/bug.h
@@ -34,9 +34,14 @@ 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, ...) __attribute__((format(printf, 3, 4)));
 #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 +53,15 @@ extern void warn_on_slowpath(const char 
 })
 #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()
@@ -63,6 +77,14 @@ extern void warn_on_slowpath(const char 
 	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)	({				\
@@ -75,6 +97,16 @@ extern void warn_on_slowpath(const char 
 	unlikely(__ret_warn_once);				\
 })
 
+#define WARN_ONCE(condition, format...)	({			\
+	static int __warned;					\
+	int __ret_warn_once = !!(condition);			\
+								\
+	if (unlikely(__ret_warn_once))				\
+		if (WARN(!__warned, format)) 			\
+			__warned = 1;				\
+	unlikely(__ret_warn_once);				\
+})
+
 #ifdef CONFIG_SMP
 # define WARN_ON_SMP(x)			WARN_ON(x)
 #else
Index: linux.trees.git/kernel/panic.c
===================================================================
--- linux.trees.git.orig/kernel/panic.c
+++ linux.trees.git/kernel/panic.c
@@ -321,6 +321,28 @@ void warn_on_slowpath(const char *file, 
 	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, caller);
+
+	printk(KERN_WARNING "------------[ cut here ]------------\n");
+	printk(KERN_WARNING "WARNING: at %s:%d %s()\n", file,
+		line, function);
+	va_start(args, fmt);
+	vprintk(fmt, args);
+	va_end(args);
+
+	print_modules();
+	dump_stack();
+	print_oops_end_marker();
+	add_taint(TAINT_WARN);
+}
+EXPORT_SYMBOL(warn_slowpath);
 #endif
 
 #ifdef CONFIG_CC_STACKPROTECTOR

Re: [patch 2/17] Add a WARN() macro that acts like WARN_ON()+printk

[Joe Perches]

On Tue, 2008-07-08 at 09:40 -0700, Arjan van de Ven wrote:
> +#ifndef WARN
> +#define WARN(condition, format...) ({					\
> +	int __ret_warn_on = !!(condition);				\
> +	if (unlikely(__ret_warn_on))					\
> +		__WARN_printf(format);					\
> +	unlikely(__ret_warn_on);					\
> +})
> +#endif
> +

If all current uses of WARN are going to change, perhaps
adding an argument for KERN_<level> or removing the
KERN_<level> prefixes and standardizing on a single
KERN_<level> (KERN_WARNING?) is appropriate.

If not standardizing on a single KERN_<level> prefix,
perhaps change the conditional and using something like:

	assert_<level>(cond, fmt, arg...)
or
	assert_msg(cond, level, fmt, arg...)

Re: [patch 2/17] Add a WARN() macro that acts like WARN_ON()+printk

[Arjan van de Ven]

On Tue, 08 Jul 2008 11:00:05 -0700
Joe Perches <joe@perches.com> wrote:

> On Tue, 2008-07-08 at 09:40 -0700, Arjan van de Ven wrote:
> > +#ifndef WARN
> > +#define WARN(condition, format...)
> > ({					\
> > +	int __ret_warn_on
> > = !!(condition);				\
> > +	if
> > (unlikely(__ret_warn_on))					\
> > +
> > __WARN_printf(format);					\
> > +
> > unlikely(__ret_warn_on);					\
> > +}) +#endif
> > +
> 
> If all current uses of WARN are going to change, perhaps
> adding an argument for KERN_<level> or removing the
> KERN_<level> prefixes and standardizing on a single
> KERN_<level> (KERN_WARNING?) is appropriate.

I looked at this and there are various levels in use today, I don't
think we can standardize on one unfortunately.
I don't think there's a real problem; WARN() really acts like printk...
all the way.



If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 2/17] Add a WARN() macro that acts like WARN_ON()+printk

[Andrew Morton]

On Tue, 8 Jul 2008 09:40:23 -0700 Arjan van de Ven <arjan@infradead.org> wrote:

> 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.
> 

Apart from a little whitespace tweak, this is identical to what I already had.

> +#define WARN_ONCE(condition, format...)	({			\
> +	static int __warned;					\
> +	int __ret_warn_once = !!(condition);			\
> +								\
> +	if (unlikely(__ret_warn_once))				\
> +		if (WARN(!__warned, format)) 			\
> +			__warned = 1;				\
> +	unlikely(__ret_warn_once);				\
> +})

Except it adds this operation, without describing it at all in the
changelog.

Is this some brainfart, or am I missing something?  I can see some sense in
a WARN_ONCE(format...), but not in a WARN_ONCE() which takes a `condition'
and should be called WARN_ON_ONCE(), which we already have.

As it appears that you didn't add any users of WARN_ONCE(), I shall
delicately step away from this patch.

More care, please?

Re: [patch 2/17] Add a WARN() macro that acts like WARN_ON()+printk

[Arjan van de Ven]

On Fri, 11 Jul 2008 12:19:49 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 8 Jul 2008 09:40:23 -0700 Arjan van de Ven
> <arjan@infradead.org> wrote:
> 
> > 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.
> > 
> 
> Apart from a little whitespace tweak, this is identical to what I
> already had.
> 
> > +#define WARN_ONCE(condition, format...)
> > ({			\
> > +	static int
> > __warned;					\
> > +	int __ret_warn_once
> > = !!(condition);			\
> > +								\
> > +	if
> > (unlikely(__ret_warn_once))				\
> > +		if (WARN(!__warned, format))
> > 			\
> > +			__warned =
> > 1;				\
> > +	unlikely(__ret_warn_once);				\
> > +})
> 
> Except it adds this operation, without describing it at all in the
> changelog.
> 
> Is this some brainfart, or am I missing something?  I can see some
> sense in a WARN_ONCE(format...), but not in a WARN_ONCE() which takes
> a `condition' and should be called WARN_ON_ONCE(), which we already
> have.

WARN_ON_ONCE() doesn't take printk arguments. So WARN_ONCE() is
WAR_ON_ONCE() with printk arguments...


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

[patch 3/17] Introduce WARN() usage in the kobject code

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>

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>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

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

Index: linux.trees.git/lib/kobject.c
===================================================================
--- linux.trees.git.orig/lib/kobject.c
+++ linux.trees.git/lib/kobject.c
@@ -215,9 +215,8 @@ static int kobject_add_internal(struct k
 		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;
 	}
 
@@ -627,12 +626,10 @@ static void kobject_release(struct 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);
 	}
 }

[patch 4/17] Use WARN() in kernel/irq/manage.c

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>

Replace a printk+WARN_ON() by a WARN(); this increases the chance of the
string making it into the bugreport (ie: it goes inside the
---[ cut here ]--- section)

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 kernel/irq/manage.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Index: linux.trees.git/kernel/irq/manage.c
===================================================================
--- linux.trees.git.orig/kernel/irq/manage.c
+++ linux.trees.git/kernel/irq/manage.c
@@ -177,8 +177,7 @@ static void __enable_irq(struct irq_desc
 {
 	switch (desc->depth) {
 	case 0:
-		printk(KERN_WARNING "Unbalanced enable for IRQ %d\n", irq);
-		WARN_ON(1);
+		WARN(1, KERN_WARNING "Unbalanced enable for IRQ %d\n", irq);
 		break;
 	case 1: {
 		unsigned int status = desc->status & ~IRQ_DISABLED;
@@ -247,9 +246,7 @@ int set_irq_wake(unsigned int irq, unsig
 			set_wake = NULL;
 	} else {
 		if (desc->wake_depth == 0) {
-			printk(KERN_WARNING "Unbalanced IRQ %d "
-					"wake disable\n", irq);
-			WARN_ON(1);
+			WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
 		} else if (--desc->wake_depth == 0)
 			desc->status &= ~IRQ_WAKEUP;
 		else

[patch 5/17] Use WARN() in kernel/panic.c

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in kernel/panic.c

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.
This allowed the if() to be folded entirely into the WARN()

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 kernel/panic.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: linux.trees.git/kernel/panic.c
===================================================================
--- linux.trees.git.orig/kernel/panic.c
+++ linux.trees.git/kernel/panic.c
@@ -392,10 +392,8 @@ static int __stack_chk_test(void)
 	printk(KERN_INFO "Testing -fstack-protector-all feature\n");
 	__stack_check_testing = (unsigned long)&__stack_chk_test_func;
 	__stack_chk_test_func();
-	if (__stack_check_testing) {
-		printk(KERN_ERR "-fstack-protector-all test failed\n");
-		WARN_ON(1);
-	}
+	WARN(__stack_check_testing,
+			KERN_ERR "-fstack-protector-all test failed\n");
 	return 0;
 }
 /*

[patch 6/17] Use WARN() in mm/vmalloc.c

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in mm/vmalloc.c

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 mm/vmalloc.c            |    6 
 2 files changed, 2 insertions(+), 309 deletions(-)

Index: linux.trees.git/mm/vmalloc.c
===================================================================
--- linux.trees.git.orig/mm/vmalloc.c
+++ linux.trees.git/mm/vmalloc.c
@@ -381,16 +381,14 @@ static void __vunmap(const void *addr, i
 		return;
 
 	if ((PAGE_SIZE-1) & (unsigned long)addr) {
-		printk(KERN_ERR "Trying to vfree() bad address (%p)\n", addr);
-		WARN_ON(1);
+		WARN(1, KERN_ERR "Trying to vfree() bad address (%p)\n", addr);
 		return;
 	}
 
 	area = remove_vm_area(addr);
 	if (unlikely(!area)) {
-		printk(KERN_ERR "Trying to vfree() nonexistent vm area (%p)\n",
+		WARN(1, KERN_ERR "Trying to vfree() nonexistent vm area (%p)\n",
 				addr);
-		WARN_ON(1);
 		return;
 	}
 

[patch 7/17] use WARN() in kernel/lockdep.c

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in kernel/lockdep.c

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 kernel/lockdep.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: linux.trees.git/kernel/lockdep.c
===================================================================
--- linux.trees.git.orig/kernel/lockdep.c
+++ linux.trees.git/kernel/lockdep.c
@@ -1643,11 +1643,10 @@ static void check_chain_key(struct task_
 		hlock = curr->held_locks + i;
 		if (chain_key != hlock->prev_chain_key) {
 			debug_locks_off();
-			printk("hm#1, depth: %u [%u], %016Lx != %016Lx\n",
+			WARN(1, "hm#1, depth: %u [%u], %016Lx != %016Lx\n",
 				curr->lockdep_depth, i,
 				(unsigned long long)chain_key,
 				(unsigned long long)hlock->prev_chain_key);
-			WARN_ON(1);
 			return;
 		}
 		id = hlock->class - lock_classes;
@@ -1662,11 +1661,10 @@ static void check_chain_key(struct task_
 	}
 	if (chain_key != curr->curr_chain_key) {
 		debug_locks_off();
-		printk("hm#2, depth: %u [%u], %016Lx != %016Lx\n",
+		WARN(1, "hm#2, depth: %u [%u], %016Lx != %016Lx\n",
 			curr->lockdep_depth, i,
 			(unsigned long long)chain_key,
 			(unsigned long long)curr->curr_chain_key);
-		WARN_ON(1);
 	}
 #endif
 }

[patch 8/17] use WARN() in kernel/irq/chip.c

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in kernel/irq

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 kernel/irq/chip.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

Index: linux.trees.git/kernel/irq/chip.c
===================================================================
--- linux.trees.git.orig/kernel/irq/chip.c
+++ linux.trees.git/kernel/irq/chip.c
@@ -28,8 +28,7 @@ void dynamic_irq_init(unsigned int irq)
 	unsigned long flags;
 
 	if (irq >= NR_IRQS) {
-		printk(KERN_ERR "Trying to initialize invalid IRQ%d\n", irq);
-		WARN_ON(1);
+		WARN(1, KERN_ERR "Trying to initialize invalid IRQ%d\n", irq);
 		return;
 	}
 
@@ -62,8 +61,7 @@ void dynamic_irq_cleanup(unsigned int ir
 	unsigned long flags;
 
 	if (irq >= NR_IRQS) {
-		printk(KERN_ERR "Trying to cleanup invalid IRQ%d\n", irq);
-		WARN_ON(1);
+		WARN(1, KERN_ERR "Trying to cleanup invalid IRQ%d\n", irq);
 		return;
 	}
 
@@ -71,9 +69,8 @@ void dynamic_irq_cleanup(unsigned int ir
 	spin_lock_irqsave(&desc->lock, flags);
 	if (desc->action) {
 		spin_unlock_irqrestore(&desc->lock, flags);
-		printk(KERN_ERR "Destroying IRQ%d without calling free_irq\n",
+		WARN(1, KERN_ERR "Destroying IRQ%d without calling free_irq\n",
 			irq);
-		WARN_ON(1);
 		return;
 	}
 	desc->msi_desc = NULL;
@@ -96,8 +93,7 @@ int set_irq_chip(unsigned int irq, struc
 	unsigned long flags;
 
 	if (irq >= NR_IRQS) {
-		printk(KERN_ERR "Trying to install chip for IRQ%d\n", irq);
-		WARN_ON(1);
+		WARN(1, KERN_ERR "Trying to install chip for IRQ%d\n", irq);
 		return -EINVAL;
 	}
 

[patch 9/17] Use WARN() in arch/x86/mm/ioremap.c

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in arch/x86/mm/ioremap.c

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Acked-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/mm/ioremap.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Index: linux.trees.git/arch/x86/mm/ioremap.c
===================================================================
--- linux.trees.git.orig/arch/x86/mm/ioremap.c
+++ linux.trees.git/arch/x86/mm/ioremap.c
@@ -522,13 +522,11 @@ static int __init check_early_ioremap_le
 {
 	if (!early_ioremap_nested)
 		return 0;
-
-	printk(KERN_WARNING
+	WARN(1, KERN_WARNING
 	       "Debug warning: early ioremap leak of %d areas detected.\n",
-	       early_ioremap_nested);
+		early_ioremap_nested);
 	printk(KERN_WARNING
-	       "please boot with early_ioremap_debug and report the dmesg.\n");
-	WARN_ON(1);
+		"please boot with early_ioremap_debug and report the dmesg.\n");
 
 	return 1;
 }

[patch 10/17] use WARN() in arch/x86/mm/pageattr.c

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in x86/mm/pageattr

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Acked-by: Ingo Molnar <mingo@elte.hu>

---
 arch/x86/mm/pageattr-test.c |    3 +--
 arch/x86/mm/pageattr.c      |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

Index: linux.trees.git/arch/x86/mm/pageattr.c
===================================================================
--- linux.trees.git.orig/arch/x86/mm/pageattr.c
+++ linux.trees.git/arch/x86/mm/pageattr.c
@@ -586,10 +586,9 @@ repeat:
 	if (!pte_val(old_pte)) {
 		if (!primary)
 			return 0;
-		printk(KERN_WARNING "CPA: called for zero pte. "
+		WARN(1, KERN_WARNING "CPA: called for zero pte. "
 		       "vaddr = %lx cpa->vaddr = %lx\n", address,
 		       cpa->vaddr);
-		WARN_ON(1);
 		return -EINVAL;
 	}
 
Index: linux.trees.git/arch/x86/mm/pageattr-test.c
===================================================================
--- linux.trees.git.orig/arch/x86/mm/pageattr-test.c
+++ linux.trees.git/arch/x86/mm/pageattr-test.c
@@ -221,8 +221,7 @@ static int pageattr_test(void)
 	failed += print_split(&sc);
 
 	if (failed) {
-		printk(KERN_ERR "NOT PASSED. Please report.\n");
-		WARN_ON(1);
+		WARN(1, KERN_ERR "NOT PASSED. Please report.\n");
 		return -EINVAL;
 	} else {
 		if (print)

[patch 11/17] Use WARN() in arch/x86/kernel

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in arch/x86/kernel

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.
This also allowed the folding of some if()'s into the WARN()

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Acked-by: Ingo Molnar <mingo@elte.hu>

---
 arch/x86/kernel/cpu/mtrr/main.c  |    5 +----
 arch/x86/kernel/pci-calgary_64.c |    3 +--
 arch/x86/kernel/tsc_sync.c       |    6 ++----
 3 files changed, 4 insertions(+), 10 deletions(-)

Index: linux.trees.git/arch/x86/kernel/pci-calgary_64.c
===================================================================
--- linux.trees.git.orig/arch/x86/kernel/pci-calgary_64.c
+++ linux.trees.git/arch/x86/kernel/pci-calgary_64.c
@@ -339,9 +339,8 @@ static void iommu_free(struct iommu_tabl
 	/* were we called with bad_dma_address? */
 	badend = bad_dma_address + (EMERGENCY_PAGES * PAGE_SIZE);
 	if (unlikely((dma_addr >= bad_dma_address) && (dma_addr < badend))) {
-		printk(KERN_ERR "Calgary: driver tried unmapping bad DMA "
+		WARN(1, KERN_ERR "Calgary: driver tried unmapping bad DMA "
 		       "address 0x%Lx\n", dma_addr);
-		WARN_ON(1);
 		return;
 	}
 
Index: linux.trees.git/arch/x86/kernel/tsc_sync.c
===================================================================
--- linux.trees.git.orig/arch/x86/kernel/tsc_sync.c
+++ linux.trees.git/arch/x86/kernel/tsc_sync.c
@@ -88,11 +88,9 @@ static __cpuinit void check_tsc_warp(voi
 			__raw_spin_unlock(&sync_lock);
 		}
 	}
-	if (!(now-start)) {
-		printk("Warning: zero tsc calibration delta: %Ld [max: %Ld]\n",
+	WARN(!(now-start),
+		"Warning: zero tsc calibration delta: %Ld [max: %Ld]\n",
 			now-start, end-start);
-		WARN_ON(1);
-	}
 }
 
 /*
Index: linux.trees.git/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
--- linux.trees.git.orig/arch/x86/kernel/cpu/mtrr/main.c
+++ linux.trees.git/arch/x86/kernel/cpu/mtrr/main.c
@@ -1496,11 +1496,8 @@ int __init mtrr_trim_uncached_memory(uns
 
 	/* kvm/qemu doesn't have mtrr set right, don't trim them all */
 	if (!highest_pfn) {
-		if (!kvm_para_available()) {
-			printk(KERN_WARNING
+		WARN(!kvm_para_available(), KERN_WARNING
 				"WARNING: strange, CPU MTRRs all blank?\n");
-			WARN_ON(1);
-		}
 		return 0;
 	}
 

[patch 12/17] Use WARN() in block/

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in block 

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>---
 block/as-iosched.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux.trees.git/block/as-iosched.c
===================================================================
--- linux.trees.git.orig/block/as-iosched.c
+++ linux.trees.git/block/as-iosched.c
@@ -837,8 +837,7 @@ static void as_completed_request(struct 
 	WARN_ON(!list_empty(&rq->queuelist));
 
 	if (RQ_STATE(rq) != AS_RQ_REMOVED) {
-		printk("rq->state %d\n", RQ_STATE(rq));
-		WARN_ON(1);
+		WARN(1, "rq->state %d\n", RQ_STATE(rq));
 		goto out;
 	}
 

[patch 13/17] Use WARN() in drivers/base/

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in drivers/base 

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>Index: linux.trees.git/drivers/base/core.c
===================================================================
---
 drivers/base/core.c   |    6 ++----
 drivers/base/memory.c |    3 +--
 drivers/base/sys.c    |   12 ++++--------
 3 files changed, 7 insertions(+), 14 deletions(-)

Index: linux.trees.git/drivers/base/core.c
===================================================================
--- linux.trees.git.orig/drivers/base/core.c
+++ linux.trees.git/drivers/base/core.c
@@ -116,12 +116,10 @@ static void device_release(struct kobjec
 		dev->type->release(dev);
 	else if (dev->class && dev->class->dev_release)
 		dev->class->dev_release(dev);
-	else {
-		printk(KERN_ERR "Device '%s' does not have a release() "
+	else
+		WARN(1, KERN_ERR "Device '%s' does not have a release() "
 			"function, it is broken and must be fixed.\n",
 			dev->bus_id);
-		WARN_ON(1);
-	}
 }
 
 static struct kobj_type device_ktype = {
Index: linux.trees.git/drivers/base/sys.c
===================================================================
--- linux.trees.git.orig/drivers/base/sys.c
+++ linux.trees.git/drivers/base/sys.c
@@ -168,19 +168,16 @@ int sysdev_driver_register(struct sysdev
 	int err = 0;
 
 	if (!cls) {
-		printk(KERN_WARNING "sysdev: invalid class passed to "
+		WARN(1, KERN_WARNING "sysdev: invalid class passed to "
 			"sysdev_driver_register!\n");
-		WARN_ON(1);
 		return -EINVAL;
 	}
 
 	/* Check whether this driver has already been added to a class. */
-	if (drv->entry.next && !list_empty(&drv->entry)) {
-		printk(KERN_WARNING "sysdev: class %s: driver (%p) has already"
+	if (drv->entry.next && !list_empty(&drv->entry))
+		WARN(1, KERN_WARNING "sysdev: class %s: driver (%p) has already"
 			" been registered to a class, something is wrong, but "
 			"will forge on!\n", cls->name, drv);
-		WARN_ON(1);
-	}
 
 	mutex_lock(&sysdev_drivers_lock);
 	if (cls && kset_get(&cls->kset)) {
@@ -194,8 +191,7 @@ int sysdev_driver_register(struct sysdev
 		}
 	} else {
 		err = -EINVAL;
-		printk(KERN_ERR "%s: invalid device class\n", __func__);
-		WARN_ON(1);
+		WARN(1, KERN_ERR "%s: invalid device class\n", __func__);
 	}
 	mutex_unlock(&sysdev_drivers_lock);
 	return err;
Index: linux.trees.git/drivers/base/memory.c
===================================================================
--- linux.trees.git.orig/drivers/base/memory.c
+++ linux.trees.git/drivers/base/memory.c
@@ -189,9 +189,8 @@ memory_block_action(struct memory_block 
 			}
 			break;
 		default:
-			printk(KERN_WARNING "%s(%p, %ld) unknown action: %ld\n",
+			WARN(1, KERN_WARNING "%s(%p, %ld) unknown action: %ld\n",
 					__func__, mem, action, action);
-			WARN_ON(1);
 			ret = -EINVAL;
 	}
 

Re: [patch 13/17] Use WARN() in drivers/base/

[Andrew Morton]

On Tue, 8 Jul 2008 09:53:07 -0700 Arjan van de Ven <arjan@infradead.org> wrote:

> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>Index: linux.trees.git/drivers/base/core.c

A number of these patches had mangled signed-off-by: lines.

Please try to be consistent in the presence and placement of the ^--- line
at the end of the changelog.

I verified that all three copies of "Use WARN() in fs/" were the same.

I've decided that I don't like the whole thing :(  This:

#define WARN(condition, format...) ({					\
	int __ret_warn_on = !!(condition);				\
	if (unlikely(__ret_warn_on))					\
		__WARN_printf(format);					\
	unlikely(__ret_warn_on);					\
})

is not a WARN().  It is a WARN_ON() function.  The use of this name now leaves
us no sensible name under which to implement

#define WARN(format...)

Re: [patch 13/17] Use WARN() in drivers/base/

[Arjan van de Ven]

On Fri, 11 Jul 2008 12:20:11 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 8 Jul 2008 09:53:07 -0700 Arjan van de Ven
> <arjan@infradead.org> wrote:
> 
> > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>Index:
> > linux.trees.git/drivers/base/core.c
> 
> A number of these patches had mangled signed-off-by: lines.
> 
> Please try to be consistent in the presence and placement of the ^---
> line at the end of the changelog.
> 
> I verified that all three copies of "Use WARN() in fs/" were the same.
> 
> I've decided that I don't like the whole thing :(  This:
> 
> #define WARN(condition, format...)
> ({					\ int __ret_warn_on
> = !!(condition);				\ if
> (unlikely(__ret_warn_on))					\
> __WARN_printf(format);					\
> unlikely(__ret_warn_on);					\ })
> 
> is not a WARN().  It is a WARN_ON() function.  The use of this name
> now leaves us no sensible name under which to implement
> 

I'm totally open to a better name.
Having a condition in there is really nice, it means we can fold the
if() into it in many cases. Just like BUG_ON() did.

Re: [patch 13/17] Use WARN() in drivers/base/

[Andrew Morton]

On Fri, 11 Jul 2008 13:54:09 -0700 Arjan van de Ven <arjan@infradead.org> wrote:

> On Fri, 11 Jul 2008 12:20:11 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Tue, 8 Jul 2008 09:53:07 -0700 Arjan van de Ven
> > <arjan@infradead.org> wrote:
> > 
> > > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>Index:
> > > linux.trees.git/drivers/base/core.c
> > 
> > A number of these patches had mangled signed-off-by: lines.
> > 
> > Please try to be consistent in the presence and placement of the ^---
> > line at the end of the changelog.
> > 
> > I verified that all three copies of "Use WARN() in fs/" were the same.
> > 
> > I've decided that I don't like the whole thing :(  This:
> > 
> > #define WARN(condition, format...)
> > ({					\ int __ret_warn_on
> > = !!(condition);				\ if
> > (unlikely(__ret_warn_on))					\
> > __WARN_printf(format);					\
> > unlikely(__ret_warn_on);					\ })
> > 
> > is not a WARN().  It is a WARN_ON() function.  The use of this name
> > now leaves us no sensible name under which to implement
> > 
> 
> I'm totally open to a better name.
> Having a condition in there is really nice, it means we can fold the
> if() into it in many cases. Just like BUG_ON() did.

Alexey's WARN_IF() would suit, I guess.  Plain old "WARN" is just wrong
here, alas.

I can just edit all the diffs if we're all OK with that.



I don't suppose there's any way of tricking the preprocessor into
supporting

	WARN_ON(foo == 42);

as well as

	WARN_ON(foo == 42, "bite me!");

Re: [patch 13/17] Use WARN() in drivers/base/

[Arjan van de Ven]

On Fri, 11 Jul 2008 15:11:10 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 11 Jul 2008 13:54:09 -0700 Arjan van de Ven
> <arjan@infradead.org> wrote:
> 
> > On Fri, 11 Jul 2008 12:20:11 -0700
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Tue, 8 Jul 2008 09:53:07 -0700 Arjan van de Ven
> > > <arjan@infradead.org> wrote:
> > > 
> > > > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>Index:
> > > > linux.trees.git/drivers/base/core.c
> > > 
> > > A number of these patches had mangled signed-off-by: lines.
> > > 
> > > Please try to be consistent in the presence and placement of the
> > > ^--- line at the end of the changelog.
> > > 
> > > I verified that all three copies of "Use WARN() in fs/" were the
> > > same.
> > > 
> > > I've decided that I don't like the whole thing :(  This:
> > > 
> > > #define WARN(condition, format...)
> > > ({					\ int __ret_warn_on
> > > = !!(condition);				\ if
> > > (unlikely(__ret_warn_on))					\
> > > __WARN_printf(format);					\
> > > unlikely(__ret_warn_on);
> > > \ })
> > > 
> > > is not a WARN().  It is a WARN_ON() function.  The use of this
> > > name now leaves us no sensible name under which to implement
> > > 
> > 
> > I'm totally open to a better name.
> > Having a condition in there is really nice, it means we can fold the
> > if() into it in many cases. Just like BUG_ON() did.
> 
> Alexey's WARN_IF() would suit, I guess.  Plain old "WARN" is just
> wrong here, alas.
> 
> I can just edit all the diffs if we're all OK with that.

I'm ok with that.

> 
> 
> 
> I don't suppose there's any way of tricking the preprocessor into
> supporting
> 
> 	WARN_ON(foo == 42);
> 
> as well as
> 
> 	WARN_ON(foo == 42, "bite me!");

there might be some obscure GCC extensions, but to be honest I fear the
worst ;(


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 13/17] Use WARN() in drivers/base/

[Arjan van de Ven]

On Fri, 11 Jul 2008 15:11:10 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> 
> I don't suppose there's any way of tricking the preprocessor into
> supporting
> 
> 	WARN_ON(foo == 42);
> 
> as well as
> 
> 	WARN_ON(foo == 42, "bite me!");
> 

after reading preprocessor docs from gcc and trying some things:
We can do this. It comes at a price: the price is a blank line in the
WARN trace for the "no printk comments" case, and we lose the ability
to override the printk level. (which you can argue is a feature by just
setting it to KERN_WARNING).

(and some interesting but otherwise non-harmful preprocessor stuff in
headers)

Is this is price worth paying to not have a second macro?

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 13/17] Use WARN() in drivers/base/

[Andrew Morton]

On Fri, 11 Jul 2008 15:51:05 -0700 Arjan van de Ven <arjan@infradead.org> wrote:

> On Fri, 11 Jul 2008 15:11:10 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > 
> > I don't suppose there's any way of tricking the preprocessor into
> > supporting
> > 
> > 	WARN_ON(foo == 42);
> > 
> > as well as
> > 
> > 	WARN_ON(foo == 42, "bite me!");
> > 
> 
> after reading preprocessor docs from gcc and trying some things:
> We can do this. It comes at a price: the price is a blank line in the
> WARN trace for the "no printk comments" case, and we lose the ability
> to override the printk level. (which you can argue is a feature by just
> setting it to KERN_WARNING).
> 
> (and some interesting but otherwise non-harmful preprocessor stuff in
> headers)

the blank line: might be avoidable by doing some extra work at runtime
to recognise its presence?

overriding facility level: doesn't sound very useful, as WARN()'s
stack-trace's facility level is not controllable.

> Is this is price worth paying to not have a second macro?

Dunno, how ugly is the patch?

It would be rather nice to not go and fatten the interface.  Would
there be additional text or data size costs?

Re: [patch 13/17] Use WARN() in drivers/base/

[Arjan van de Ven]

On Fri, 11 Jul 2008 16:02:13 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 11 Jul 2008 15:51:05 -0700 Arjan van de Ven
> <arjan@infradead.org> wrote:
> 
> > On Fri, 11 Jul 2008 15:11:10 -0700
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > 
> > > I don't suppose there's any way of tricking the preprocessor into
> > > supporting
> > > 
> > > 	WARN_ON(foo == 42);
> > > 
> > > as well as
> > > 
> > > 	WARN_ON(foo == 42, "bite me!");
> > > 
> > 
> > after reading preprocessor docs from gcc and trying some things:
> > We can do this. It comes at a price: the price is a blank line in
> > the WARN trace for the "no printk comments" case, and we lose the
> > ability to override the printk level. (which you can argue is a
> > feature by just setting it to KERN_WARNING).
> > 
> > (and some interesting but otherwise non-harmful preprocessor stuff
> > in headers)
> 
> the blank line: might be avoidable by doing some extra work at runtime
> to recognise its presence?

probably (but vararg stuff is weird)

> 
> overriding facility level: doesn't sound very useful, as WARN()'s
> stack-trace's facility level is not controllable.

ok

> 
> > Is this is price worth paying to not have a second macro?
> 
> Dunno, how ugly is the patch?

it's not too bad ;) I'll turn the userland experiment into a kernel
patch tomorrow or so

> 
> It would be rather nice to not go and fatten the interface.  Would
> there be additional text or data size costs?

there will be a few bytes of text in the out of line implementation;
I'm not too worried about that.
There shouldn't be a per-instance overhead

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 13/17] Use WARN() in drivers/base/

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]


> I don't suppose there's any way of tricking the preprocessor into
> supporting
> 
> 	WARN_ON(foo == 42);
> 
> as well as
> 
> 	WARN_ON(foo == 42, "bite me!");

Here's one that abuses variadic macros and limits the number of
arguments to 19.

#include <stdarg.h>
#include <stdio.h>

#define cnt(y...)   _cnt( , ## y)
#define _cnt(y...) __cnt(y,19,18,17,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,1,0)
#define __cnt(x0,x1,x2,x3,x4,x5,x6,x7,x8,x9,x10,x11,x12,x13,x14,x15,x16,x17,x18,x19,n,ys...) n

static void warn_on_slowpath(int argcount, ...)
{
        va_list args;
        char *fmt;

	// print beginning of warning

        if (argcount) {
	        va_start(args, count);
        	fmt = va_arg(args, char *);
	        vprintf(fmt, args);
	        va_end(args);
	}

	// print rest of warning
}

#define WARN_ON(test, fmt...)           	\
  do {                                  	\
      if (test) {                       	\
          warn_on_slowpath(cnt(fmt) , ## fmt);	\
          printf("WARN\n");             	\
      }                                 	\
  } while (0)

int main(void)
{
        WARN_ON(1);
        WARN_ON(1, "asdf %d\n", 7);

        return 0;
}


johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch 13/17] Use WARN() in drivers/base/

[Johannes Berg]

> #define WARN_ON(test, fmt...)           	\
>   do {                                  	\
>       if (test) {                       	\
>           warn_on_slowpath(cnt(fmt) , ## fmt);	\
>           printf("WARN\n");             	\
>       }                                 	\
>   } while (0)

That doesn't play well with the bugtrap stuff, obviously. This one is a
bit better, but still has more overhead than the bugtrap stuff, but
avoids the overhead in the no-argument case.


#include <stdarg.h>
#include <stdio.h>

#define unlikely(x) (x)

#define hasargs(...) _hasargs( , ## __VA_ARGS__)
#define _hasargs(...) __hasargs(__VA_ARGS__,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,0)
#define __hasargs(x0,x1,x2,x3,x4,x5,x6,x7,x8,x9,x10,x11,x12,x13,x14,x15,x16,x17,x18,x19,n,ys...) n

static void warn_slowpath(int count, ...)
{
	va_list args;
	char *fmt;

	printf("WARNING: <<\n");

	if (count) {
		va_start(args, count);
		fmt = va_arg(args, char *);
		vprintf(fmt, args);
		va_end(args);
	}
	printf(">>\n");
}

/*
 * This is the original arch WARN_ON that just emits
 * a trap instruction and a bugtrap section entry for
 * example on powerpc.
 *
 * Obviously this isn't, but it could be.
 */
#define BUGTRAP_WARN_ON(test) ({			\
	if (unlikely(test))				\
		printf("WARNING: %s\n", #test);		\
	unlikely(test);					\
})

/*
 * This is a WARN_ON for some other architectures that don't
 * use the bugtrap section, it calls the slowpath function
 * which leads to more code size. But when you've added
 * parameters to your WARN_ON those need to be calculated
 * as well so you've already increased code size.
 */
#define OUT_OF_LINE_WARN_ON(test, fmt...) ({		\
	if (unlikely(test))				\
		warn_slowpath(hasargs(fmt) , ## fmt);	\
	unlikely(test);					\
})

/*
 * This new version determines whether to do a bugtrap
 * entry or not depending on whether there were any extra
 * arguments given.
 */
#define CONFIG_BUG_EXTRA_VERBOSE
#ifdef CONFIG_BUG_EXTRA_VERBOSE
#define WARN_ON(test, fmt...) ({			\
	if (!hasargs(fmt))				\
		BUGTRAP_WARN_ON(test);			\
	else						\
		OUT_OF_LINE_WARN_ON(test , ## fmt);	\
})
#else
#define WARN_ON(test, fmt...) BUGTRAP_WARN_ON(test)
#endif

int main(void)
{
	WARN_ON(1);
	WARN_ON(2, "asdf %d %d\n", 7, 8);
	WARN_ON(3, "asdf\n");

	return 0;
}

[patch 14/17] Use WARN() in lib/

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in /lib 

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.
In addition, one of the if() clauses collapes into the WARN() entirely
now.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>---
 lib/debugobjects.c   |   15 +++++----------
 lib/iomap.c          |    3 +--
 lib/kobject_uevent.c |    6 ++----
 lib/plist.c          |   13 +++++++------
 4 files changed, 15 insertions(+), 22 deletions(-)

Index: linux.trees.git/lib/debugobjects.c
===================================================================
--- linux.trees.git.orig/lib/debugobjects.c
+++ linux.trees.git/lib/debugobjects.c
@@ -205,9 +205,8 @@ static void debug_print_object(struct de
 
 	if (limit < 5 && obj->descr != descr_test) {
 		limit++;
-		printk(KERN_ERR "ODEBUG: %s %s object type: %s\n", msg,
+		WARN(1, KERN_ERR "ODEBUG: %s %s object type: %s\n", msg,
 		       obj_states[obj->state], obj->descr->name);
-		WARN_ON(1);
 	}
 	debug_objects_warnings++;
 }
@@ -735,26 +734,22 @@ check_results(void *addr, enum debug_obj
 
 	obj = lookup_object(addr, db);
 	if (!obj && state != ODEBUG_STATE_NONE) {
-		printk(KERN_ERR "ODEBUG: selftest object not found\n");
-		WARN_ON(1);
+		WARN(1, KERN_ERR "ODEBUG: selftest object not found\n");
 		goto out;
 	}
 	if (obj && obj->state != state) {
-		printk(KERN_ERR "ODEBUG: selftest wrong state: %d != %d\n",
+		WARN(1, KERN_ERR "ODEBUG: selftest wrong state: %d != %d\n",
 		       obj->state, state);
-		WARN_ON(1);
 		goto out;
 	}
 	if (fixups != debug_objects_fixups) {
-		printk(KERN_ERR "ODEBUG: selftest fixups failed %d != %d\n",
+		WARN(1, KERN_ERR "ODEBUG: selftest fixups failed %d != %d\n",
 		       fixups, debug_objects_fixups);
-		WARN_ON(1);
 		goto out;
 	}
 	if (warnings != debug_objects_warnings) {
-		printk(KERN_ERR "ODEBUG: selftest warnings failed %d != %d\n",
+		WARN(1, KERN_ERR "ODEBUG: selftest warnings failed %d != %d\n",
 		       warnings, debug_objects_warnings);
-		WARN_ON(1);
 		goto out;
 	}
 	res = 0;
Index: linux.trees.git/lib/iomap.c
===================================================================
--- linux.trees.git.orig/lib/iomap.c
+++ linux.trees.git/lib/iomap.c
@@ -40,8 +40,7 @@ static void bad_io_access(unsigned long 
 	static int count = 10;
 	if (count) {
 		count--;
-		printk(KERN_ERR "Bad IO access at port %#lx (%s)\n", port, access);
-		WARN_ON(1);
+		WARN(1, KERN_ERR "Bad IO access at port %#lx (%s)\n", port, access);
 	}
 }
 
Index: linux.trees.git/lib/kobject_uevent.c
===================================================================
--- linux.trees.git.orig/lib/kobject_uevent.c
+++ linux.trees.git/lib/kobject_uevent.c
@@ -285,8 +285,7 @@ int add_uevent_var(struct kobj_uevent_en
 	int len;
 
 	if (env->envp_idx >= ARRAY_SIZE(env->envp)) {
-		printk(KERN_ERR "add_uevent_var: too many keys\n");
-		WARN_ON(1);
+		WARN(1, KERN_ERR "add_uevent_var: too many keys\n");
 		return -ENOMEM;
 	}
 
@@ -297,8 +296,7 @@ int add_uevent_var(struct kobj_uevent_en
 	va_end(args);
 
 	if (len >= (sizeof(env->buf) - env->buflen)) {
-		printk(KERN_ERR "add_uevent_var: buffer size too small\n");
-		WARN_ON(1);
+		WARN(1, KERN_ERR "add_uevent_var: buffer size too small\n");
 		return -ENOMEM;
 	}
 
Index: linux.trees.git/lib/plist.c
===================================================================
--- linux.trees.git.orig/lib/plist.c
+++ linux.trees.git/lib/plist.c
@@ -31,12 +31,13 @@
 static void plist_check_prev_next(struct list_head *t, struct list_head *p,
 				  struct list_head *n)
 {
-	if (n->prev != p || p->next != n) {
-		printk("top: %p, n: %p, p: %p\n", t, t->next, t->prev);
-		printk("prev: %p, n: %p, p: %p\n", p, p->next, p->prev);
-		printk("next: %p, n: %p, p: %p\n", n, n->next, n->prev);
-		WARN_ON(1);
-	}
+	WARN(n->prev != p || p->next != n,
+			"top: %p, n: %p, p: %p\n"
+			"prev: %p, n: %p, p: %p\n"
+			"next: %p, n: %p, p: %p\n",
+			 t, t->next, t->prev,
+			p, p->next, p->prev,
+			n, n->next, n->prev);
 }
 
 static void plist_check_list(struct list_head *top)

[patch 15/17] Use WARN() in fs/

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in fs/

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 fs/buffer.c    |    3 +--
 fs/namespace.c |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

Index: linux.trees.git/fs/buffer.c
===================================================================
--- linux.trees.git.orig/fs/buffer.c
+++ linux.trees.git/fs/buffer.c
@@ -1214,8 +1214,7 @@ void __brelse(struct buffer_head * buf)
 		put_bh(buf);
 		return;
 	}
-	printk(KERN_ERR "VFS: brelse: Trying to free free buffer\n");
-	WARN_ON(1);
+	WARN(1, KERN_ERR "VFS: brelse: Trying to free free buffer\n");
 }
 
 /*
Index: linux.trees.git/fs/namespace.c
===================================================================
--- linux.trees.git.orig/fs/namespace.c
+++ linux.trees.git/fs/namespace.c
@@ -309,10 +309,9 @@ static void handle_write_count_underflow
 	 */
 	if ((atomic_read(&mnt->__mnt_writers) < 0) &&
 	    !(mnt->mnt_flags & MNT_IMBALANCED_WRITE_COUNT)) {
-		printk(KERN_DEBUG "leak detected on mount(%p) writers "
+		WARN(1, KERN_DEBUG "leak detected on mount(%p) writers "
 				"count: %d\n",
 			mnt, atomic_read(&mnt->__mnt_writers));
-		WARN_ON(1);
 		/* use the flag to keep the dmesg spam down */
 		mnt->mnt_flags |= MNT_IMBALANCED_WRITE_COUNT;
 	}

[patch 15/17] Use WARN() in fs/

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in fs/

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 fs/buffer.c    |    3 +--
 fs/namespace.c |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

Index: linux.trees.git/fs/buffer.c
===================================================================
--- linux.trees.git.orig/fs/buffer.c
+++ linux.trees.git/fs/buffer.c
@@ -1214,8 +1214,7 @@ void __brelse(struct buffer_head * buf)
 		put_bh(buf);
 		return;
 	}
-	printk(KERN_ERR "VFS: brelse: Trying to free free buffer\n");
-	WARN_ON(1);
+	WARN(1, KERN_ERR "VFS: brelse: Trying to free free buffer\n");
 }
 
 /*
Index: linux.trees.git/fs/namespace.c
===================================================================
--- linux.trees.git.orig/fs/namespace.c
+++ linux.trees.git/fs/namespace.c
@@ -309,10 +309,9 @@ static void handle_write_count_underflow
 	 */
 	if ((atomic_read(&mnt->__mnt_writers) < 0) &&
 	    !(mnt->mnt_flags & MNT_IMBALANCED_WRITE_COUNT)) {
-		printk(KERN_DEBUG "leak detected on mount(%p) writers "
+		WARN(1, KERN_DEBUG "leak detected on mount(%p) writers "
 				"count: %d\n",
 			mnt, atomic_read(&mnt->__mnt_writers));
-		WARN_ON(1);
 		/* use the flag to keep the dmesg spam down */
 		mnt->mnt_flags |= MNT_IMBALANCED_WRITE_COUNT;
 	}

[patch 15/17] Use WARN() in fs/

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN instead of printk+WARN_ON in fs/

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 fs/buffer.c    |    3 +--
 fs/namespace.c |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

Index: linux.trees.git/fs/buffer.c
===================================================================
--- linux.trees.git.orig/fs/buffer.c
+++ linux.trees.git/fs/buffer.c
@@ -1214,8 +1214,7 @@ void __brelse(struct buffer_head * buf)
 		put_bh(buf);
 		return;
 	}
-	printk(KERN_ERR "VFS: brelse: Trying to free free buffer\n");
-	WARN_ON(1);
+	WARN(1, KERN_ERR "VFS: brelse: Trying to free free buffer\n");
 }
 
 /*
Index: linux.trees.git/fs/namespace.c
===================================================================
--- linux.trees.git.orig/fs/namespace.c
+++ linux.trees.git/fs/namespace.c
@@ -309,10 +309,9 @@ static void handle_write_count_underflow
 	 */
 	if ((atomic_read(&mnt->__mnt_writers) < 0) &&
 	    !(mnt->mnt_flags & MNT_IMBALANCED_WRITE_COUNT)) {
-		printk(KERN_DEBUG "leak detected on mount(%p) writers "
+		WARN(1, KERN_DEBUG "leak detected on mount(%p) writers "
 				"count: %d\n",
 			mnt, atomic_read(&mnt->__mnt_writers));
-		WARN_ON(1);
 		/* use the flag to keep the dmesg spam down */
 		mnt->mnt_flags |= MNT_IMBALANCED_WRITE_COUNT;
 	}

[patch 16/17] Usr WARN() in fs/sysfs

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN() instead of printk+WARN_ON in fs/sysfs

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.
Also, with this, one fo the if() sections collapses entirely into
the WARN().

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 fs/sysfs/dir.c   |    5 +----
 fs/sysfs/file.c  |    3 +--
 fs/sysfs/group.c |    3 +--
 3 files changed, 3 insertions(+), 8 deletions(-)

Index: linux.trees.git/fs/sysfs/dir.c
===================================================================
--- linux.trees.git.orig/fs/sysfs/dir.c
+++ linux.trees.git/fs/sysfs/dir.c
@@ -459,11 +459,8 @@ int sysfs_add_one(struct sysfs_addrm_cxt
 	int ret;
 
 	ret = __sysfs_add_one(acxt, sd);
-	if (ret == -EEXIST) {
-		printk(KERN_WARNING "sysfs: duplicate filename '%s' "
+	WARN(ret == -EEXIST, KERN_WARNING "sysfs: duplicate filename '%s' "
 		       "can not be created\n", sd->s_name);
-		WARN_ON(1);
-	}
 	return ret;
 }
 
Index: linux.trees.git/fs/sysfs/file.c
===================================================================
--- linux.trees.git.orig/fs/sysfs/file.c
+++ linux.trees.git/fs/sysfs/file.c
@@ -350,9 +350,8 @@ static int sysfs_open_file(struct inode 
 	if (kobj->ktype && kobj->ktype->sysfs_ops)
 		ops = kobj->ktype->sysfs_ops;
 	else {
-		printk(KERN_ERR "missing sysfs attribute operations for "
+		WARN(1, KERN_ERR "missing sysfs attribute operations for "
 		       "kobject: %s\n", kobject_name(kobj));
-		WARN_ON(1);
 		goto err_out;
 	}
 
Index: linux.trees.git/fs/sysfs/group.c
===================================================================
--- linux.trees.git.orig/fs/sysfs/group.c
+++ linux.trees.git/fs/sysfs/group.c
@@ -134,9 +134,8 @@ void sysfs_remove_group(struct kobject *
 	if (grp->name) {
 		sd = sysfs_get_dirent(dir_sd, grp->name);
 		if (!sd) {
-			printk(KERN_WARNING "sysfs group %p not found for "
+			WARN(!sd, KERN_WARNING "sysfs group %p not found for "
 				"kobject '%s'\n", grp, kobject_name(kobj));
-			WARN_ON(!sd);
 			return;
 		}
 	} else

[patch 17/17] Use WARN() in fs/proc/

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Use WARN() instead of printk+WARN_ON() in fs/procfs

Use WARN() instead of a printk+WARN_ON() pair; this way the message
becomes part of the warning section for better reporting/collection.
This way, the entire if() {} section can collapse into the WARN() as well.

Signed-off-by: Arjan van de VenIndex: linux.trees.git/fs/proc/generic.c
===================================================================
---
 fs/proc/generic.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Index: linux.trees.git/fs/proc/generic.c
===================================================================
--- linux.trees.git.orig/fs/proc/generic.c
+++ linux.trees.git/fs/proc/generic.c
@@ -792,12 +792,9 @@ continue_removing:
 	if (S_ISDIR(de->mode))
 		parent->nlink--;
 	de->nlink = 0;
-	if (de->subdir) {
-		printk(KERN_WARNING "%s: removing non-empty directory "
+	WARN(de->subdir, KERN_WARNING "%s: removing non-empty directory "
 			"'%s/%s', leaking at least '%s'\n", __func__,
 			de->parent->name, de->name, de->subdir->name);
-		WARN_ON(1);
-	}
 	if (atomic_dec_and_test(&de->count))
 		free_proc_entry(de);
 }

Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

[Ingo Molnar]

* Arjan van de Ven <arjan@infradead.org> wrote:

> Hi,
> 
> This patch series introduces WARN(), which is a WARN_ON() variant that 
> takes printk() like arguments. This was done after both akpm and I hit 
> several cases where this would have been useful; in addition, with 
> WARN(), we put the printk string inside the -----[ cut here ]----- 
> section, making it more likely that users (and tools like kerneloops) 
> pick up the message in addition to just the bare WARNING.
> 
> The first few patches have been in -mm for a long time; the later ones 
> are newer and introduce more users of WARN().

i've created a new -git based topic branch in tip/core/warn-API and 
picked up your patches:

 Arjan van de Ven (16):
      full tree: clear the WARN() namespace
      core: add a WARN() macro that acts like WARN_ON()+printk
      kobject: introduce WARN() usage in the kobject code
      irq: use WARN() in kernel/irq/manage.c
      mm: use WARN() in mm/vmalloc.c
      lockdep: use WARN() in kernel/lockdep.c
      irq: use WARN() in kernel/irq/chip.c
      x86: use WARN() in arch/x86/mm/ioremap.c
      x86: use WARN() in arch/x86/mm/pageattr.c
      x86: use WARN() in arch/x86/kernel
      block: use WARN() in block/
      driver core: use WARN() in drivers/base/
      core libs: use WARN() in lib/
      fs: use WARN() in fs/
      sysfs: use WARN() in fs/sysfs
      procfs: use WARN() in fs/proc/

it can be picked up via:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/warn-API

( Note, i added a few more tree-wide namespace preparation fixes to the 
  first patch - a few more came in since you first prepared this patch i 
  guess. )

	Ingo

Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

[Andrew Morton]

On Wed, 9 Jul 2008 12:13:48 +0200 Ingo Molnar <mingo@elte.hu> wrote:

> > Hi,
> > 
> > This patch series introduces WARN(), which is a WARN_ON() variant that 
> > takes printk() like arguments. This was done after both akpm and I hit 
> > several cases where this would have been useful; in addition, with 
> > WARN(), we put the printk string inside the -----[ cut here ]----- 
> > section, making it more likely that users (and tools like kerneloops) 
> > pick up the message in addition to just the bare WARNING.
> > 
> > The first few patches have been in -mm for a long time; the later ones 
> > are newer and introduce more users of WARN().
> 
> i've created a new -git based topic branch in tip/core/warn-API and 
> picked up your patches:

um, why?

If you merge this into linux-next then it will trash already-merged patches
in -mm and, more particularly, it will trash other trees which you aren't
looking at, causing Stephen problems.

The way to merge this code is to get the base patches into mainline and
then trickle the dependent patches into subsystem trees, or direct into
mainline after the subsystem trees have merged, and with suitable acks.

You aren't set up to do that?

Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> > > The first few patches have been in -mm for a long time; the later 
> > > ones are newer and introduce more users of WARN().
> > 
> > i've created a new -git based topic branch in tip/core/warn-API and 
> > picked up your patches:
> 
> um, why?
> 
> If you merge this into linux-next then it will trash already-merged 
> patches in -mm and, more particularly, it will trash other trees which 
> you aren't looking at, causing Stephen problems.

no, i didnt plan to push this towards linux-next - given the broad 
consensus and given the wide spread of the changes.

I wanted to wait with this until the end of the merge window and keep it 
tested and merged up nicely. I.e. zero maintenance overhead to 
subsystems.

> The way to merge this code is to get the base patches into mainline 
> and then trickle the dependent patches into subsystem trees, or direct 
> into mainline after the subsystem trees have merged, and with suitable 
> acks.
> 
> You aren't set up to do that?

i think it's better to just go through the merge window i believe, and 
then do this atomically in one correct and tested step, when all 
subsystem trees are at their minimum size and there's virtually no 
collisions.

Note that this situation is special: this is a patchset that has 
virtually no functionality side-effects, and hence can be done 100% 
correctly, i thought the atomic step was the right approach.

For anything semantically meaningful i too would do the spread-out 
gradual approach (and i'm presently doing that for a number of topics).

But if you'd like to do this the spread-out way then sure, and i will 
drop this tree. ( if you do that then please import the commits from 
tip/core/warn-API, i fixed a couple of of typos in the commit messages 
and did some merging and extensions as well. The tree also passed a fair 
amount of testing meanwhile as well. )

Anyway, your call.

	Ingo

Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

[Andrew Morton]

On Wed, 9 Jul 2008 13:37:03 +0200 Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > > > The first few patches have been in -mm for a long time; the later 
> > > > ones are newer and introduce more users of WARN().
> > > 
> > > i've created a new -git based topic branch in tip/core/warn-API and 
> > > picked up your patches:
> > 
> > um, why?
> > 
> > If you merge this into linux-next then it will trash already-merged 
> > patches in -mm and, more particularly, it will trash other trees which 
> > you aren't looking at, causing Stephen problems.
> 
> no, i didnt plan to push this towards linux-next - given the broad 
> consensus and given the wide spread of the changes.

Well, there was no way for me (or, I believe, Arjan or anyone else) to have
worked this out from your reply.

> I wanted to wait with this until the end of the merge window and keep it 
> tested and merged up nicely. I.e. zero maintenance overhead to 
> subsystems.

That's an option.  That's why I will cc the relevant subsystem maintainers
on the commits, and will collect and maintain the acked-by's.

> > The way to merge this code is to get the base patches into mainline 
> > and then trickle the dependent patches into subsystem trees, or direct 
> > into mainline after the subsystem trees have merged, and with suitable 
> > acks.
> > 
> > You aren't set up to do that?
> 
> i think it's better to just go through the merge window i believe, and 
> then do this atomically in one correct and tested step, when all 
> subsystem trees are at their minimum size and there's virtually no 
> collisions.

Probably.

> Note that this situation is special: this is a patchset that has 
> virtually no functionality side-effects, and hence can be done 100% 
> correctly, i thought the atomic step was the right approach.
> 
> For anything semantically meaningful i too would do the spread-out 
> gradual approach (and i'm presently doing that for a number of topics).
> 
> But if you'd like to do this the spread-out way then sure, and i will 
> drop this tree. ( if you do that then please import the commits from 
> tip/core/warn-API, i fixed a couple of of typos in the commit messages 
> and did some merging and extensions as well. The tree also passed a fair 
> amount of testing meanwhile as well. )
> 
> Anyway, your call.

Well I haven't got onto processing these patches in detail yet.  An open
questions is why the damn thing was resubmitted from scratch when I've
already merged it and fixed various rejects and had to fix several bugs in
it.  Do those rejects need to be re-fixed?  Were my bugfixes folded back? 
I haven't looked yet.  I'll need to generate the incremental diff and see
what was done.

But if what you've merged was against mainline then it isn't terribly
useful.

Hopefully this sort of thing won't happen as much once I get -mm into
linx-next.  Soon...

Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> > Note that this situation is special: this is a patchset that has 
> > virtually no functionality side-effects, and hence can be done 100% 
> > correctly, i thought the atomic step was the right approach.
> > 
> > For anything semantically meaningful i too would do the spread-out 
> > gradual approach (and i'm presently doing that for a number of 
> > topics).
> > 
> > But if you'd like to do this the spread-out way then sure, and i 
> > will drop this tree. ( if you do that then please import the commits 
> > from tip/core/warn-API, i fixed a couple of of typos in the commit 
> > messages and did some merging and extensions as well. The tree also 
> > passed a fair amount of testing meanwhile as well. )
> > 
> > Anyway, your call.
> 
> Well I haven't got onto processing these patches in detail yet.  An 
> open questions is why the damn thing was resubmitted from scratch when 
> I've already merged it and fixed various rejects and had to fix 
> several bugs in it.  Do those rejects need to be re-fixed?  Were my 
> bugfixes folded back? I haven't looked yet.  I'll need to generate the 
> incremental diff and see what was done.
> 
> But if what you've merged was against mainline then it isn't terribly 
> useful.

i cross-ported it from the linux-next base to -git base and the conflict 
fallout was minimal, well below 5% or so. I.e. the patches change 
long-existent warnings that have not changed in linux-next either.

About 30% of the changes in this patchset affect subsystems that i 
co-maintain, still tip/core/warn-API did a pure, conflict-free git-merge 
when it was applied ontop of all these subsystem trees:

 commit 99eb83efbe2e3c12d26be22a032495ccddb36a2c
 Merge: de67579... c2e0195...
 Author: Ingo Molnar <mingo@elte.hu>
 Date:   Wed Jul 9 12:14:48 2008 +0200

     Merge branch 'core/warn-API'

Hence my suggestion that this should be maintained and forward ported to 
the end of the merge window, in a separate, standalone tree that is -git 
based.

Anyway, no strong feelings, it's your call.

	Ingo

Re: [patch 0/17] Series to introduce WARN()... a WARN_ON() variant that takes printk arguments

[Arjan van de Ven]

On Wed, 9 Jul 2008 04:46:13 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> Well I haven't got onto processing these patches in detail yet.  An
> open questions is why the damn thing was resubmitted from scratch
> when I've already merged it and fixed various rejects and had to fix
> several bugs in it.  Do those rejects need to be re-fixed?  Were my
> bugfixes folded back? I haven't looked yet.  I'll need to generate
> the incremental diff and see what was done.

I started with the version in -mm (and had to do one reject fix against
-next) with folding the 2 -fix's into it.

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org