[PATCH 1/6] Staging: hv: vmbus_drv.c Replaced DPRINT with native pr_XXX

[PATCH 1/6] Staging: hv: vmbus_drv.c Replaced DPRINT with native pr_XXX

[Hank Janssen]

This group of patches removes all DPRINT from hv_vmbus.ko.
It is divided in several patches due to size.

All DPRINT calls have been removed, and where needed have been
replaced with pr_XX native calls. Many debug DPRINT calls have
been removed outright.

The amount of clutter this driver prints has been 
significantly reduced.

Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/staging/hv/logging.h   |    1 +
 drivers/staging/hv/vmbus_drv.c |  145 +++++++++++-----------------------------
 2 files changed, 39 insertions(+), 107 deletions(-)

diff --git a/drivers/staging/hv/logging.h b/drivers/staging/hv/logging.h
index 1799951..517d721 100644
--- a/drivers/staging/hv/logging.h
+++ b/drivers/staging/hv/logging.h
@@ -31,6 +31,7 @@
 /* #include <linux/init.h> */
 /* #include <linux/module.h> */
 
+#define VMBUS_MOD                       "hv_vmbus"
 
 #define VMBUS				0x0001
 #define STORVSC				0x0002
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 459c707..a560a80 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -279,22 +279,16 @@ static int vmbus_on_isr(struct hv_driver *drv)
 	msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
 
 	/* Check if there are actual msgs to be process */
-	if (msg->header.message_type != HVMSG_NONE) {
-		DPRINT_DBG(VMBUS, "received msg type %d size %d",
-				msg->header.message_type,
-				msg->header.payload_size);
+	if (msg->header.message_type != HVMSG_NONE)
 		ret |= 0x1;
-	}
 
 	/* TODO: Check if there are events to be process */
 	page_addr = hv_context.synic_event_page[cpu];
 	event = (union hv_synic_event_flags *)page_addr + VMBUS_MESSAGE_SINT;
 
 	/* Since we are a child, we only need to check bit 0 */
-	if (test_and_clear_bit(0, (unsigned long *) &event->flags32[0])) {
-		DPRINT_DBG(VMBUS, "received event %d", event->flags32[0]);
+	if (test_and_clear_bit(0, (unsigned long *) &event->flags32[0]))
 		ret |= 0x2;
-	}
 
 	return ret;
 }
@@ -468,17 +462,6 @@ static int vmbus_bus_init(void)
 	int ret;
 	unsigned int vector;
 
-	DPRINT_INFO(VMBUS, "+++++++ HV Driver version = %s +++++++",
-		    HV_DRV_VERSION);
-	DPRINT_INFO(VMBUS, "+++++++ Vmbus supported version = %d +++++++",
-			VMBUS_REVISION_NUMBER);
-	DPRINT_INFO(VMBUS, "+++++++ Vmbus using SINT %d +++++++",
-			VMBUS_MESSAGE_SINT);
-	DPRINT_DBG(VMBUS, "sizeof(vmbus_channel_packet_page_buffer)=%zd, "
-			"sizeof(VMBUS_CHANNEL_PACKET_MULITPAGE_BUFFER)=%zd",
-			sizeof(struct vmbus_channel_packet_page_buffer),
-			sizeof(struct vmbus_channel_packet_multipage_buffer));
-
 	driver->name = driver_name;
 	memcpy(&driver->dev_type, &device_type, sizeof(struct hv_guid));
 
@@ -490,15 +473,8 @@ static int vmbus_bus_init(void)
 	/* Hypervisor initialization...setup hypercall page..etc */
 	ret = hv_init();
 	if (ret != 0) {
-		DPRINT_ERR(VMBUS, "Unable to initialize the hypervisor - 0x%x",
-				ret);
-		goto cleanup;
-	}
-
-	/* Sanity checks */
-	if (!driver->dev_add) {
-		DPRINT_ERR(VMBUS_DRV, "OnDeviceAdd() routine not set");
-		ret = -1;
+		pr_err("%s: %s - Unable to initialize hypervisor - 0x%x",
+		       VMBUS_MOD, __func__, ret);
 		goto cleanup;
 	}
 
@@ -522,8 +498,8 @@ static int vmbus_bus_init(void)
 			  driver->name, NULL);
 
 	if (ret != 0) {
-		DPRINT_ERR(VMBUS_DRV, "ERROR - Unable to request IRQ %d",
-			   vmbus_irq);
+		pr_err("%s: %s ERROR - Unable to request IRQ %d",
+			VMBUS_MOD, __func__, vmbus_irq);
 
 		bus_unregister(&vmbus_drv_ctx->bus);
 
@@ -532,15 +508,15 @@ static int vmbus_bus_init(void)
 	}
 	vector = VMBUS_IRQ_VECTOR;
 
-	DPRINT_INFO(VMBUS_DRV, "irq 0x%x vector 0x%x", vmbus_irq, vector);
+	pr_info("%s: irq 0x%x vector 0x%x", VMBUS_MOD, vmbus_irq, vector);
 
 	/* Call to bus driver to add the root device */
 	memset(dev_ctx, 0, sizeof(struct vm_device));
 
 	ret = driver->dev_add(&dev_ctx->device_obj, &vector);
 	if (ret != 0) {
-		DPRINT_ERR(VMBUS_DRV,
-			   "ERROR - Unable to add vmbus root device");
+		pr_err("%s: %s ERROR - Unable to add hv_vmbus root device",
+		       VMBUS_MOD, __func__);
 
 		free_irq(vmbus_irq, NULL);
 
@@ -567,8 +543,8 @@ static int vmbus_bus_init(void)
 	/* Setup the bus as root device */
 	ret = device_register(&dev_ctx->device);
 	if (ret) {
-		DPRINT_ERR(VMBUS_DRV,
-			   "ERROR - Unable to register vmbus root device");
+		pr_err("%s: %s ERROR Unable to register vmbus root device",
+		       VMBUS_MOD, __func__);
 
 		free_irq(vmbus_irq, NULL);
 		bus_unregister(&vmbus_drv_ctx->bus);
@@ -631,9 +607,6 @@ int vmbus_child_driver_register(struct driver_context *driver_ctx)
 {
 	int ret;
 
-	DPRINT_INFO(VMBUS_DRV, "child driver (%p) registering - name %s",
-		    driver_ctx, driver_ctx->driver.name);
-
 	/* The child driver on this vmbus */
 	driver_ctx->driver.bus = &vmbus_drv.bus;
 
@@ -641,6 +614,13 @@ int vmbus_child_driver_register(struct driver_context *driver_ctx)
 
 	vmbus_request_offers();
 
+	if (ret)
+		pr_err("%s: %s Unable to register Hyper-V driver %s",
+		       VMBUS_MOD, __func__, driver_ctx->driver.name);
+	else
+		pr_info("%s: Hyper-V driver registering %s", VMBUS_MOD,
+			driver_ctx->driver.name);
+
 	return ret;
 }
 EXPORT_SYMBOL(vmbus_child_driver_register);
@@ -658,11 +638,11 @@ EXPORT_SYMBOL(vmbus_child_driver_register);
  */
 void vmbus_child_driver_unregister(struct driver_context *driver_ctx)
 {
-	DPRINT_INFO(VMBUS_DRV, "child driver (%p) unregistering - name %s",
-		    driver_ctx, driver_ctx->driver.name);
-
 	driver_unregister(&driver_ctx->driver);
 
+	pr_info("%s: child driver unregistering - %s", VMBUS_MOD,
+		driver_ctx->driver.name);
+
 	driver_ctx->driver.bus = NULL;
 }
 EXPORT_SYMBOL(vmbus_child_driver_unregister);
@@ -681,30 +661,11 @@ struct hv_device *vmbus_child_device_create(struct hv_guid *type,
 	/* Allocate the new child device */
 	child_device_ctx = kzalloc(sizeof(struct vm_device), GFP_KERNEL);
 	if (!child_device_ctx) {
-		DPRINT_ERR(VMBUS_DRV,
-			"unable to allocate device_context for child device");
+		pr_err("%s: %s Unable to allocate device_context child device",
+		       VMBUS_MOD, __func__);
 		return NULL;
 	}
 
-	DPRINT_DBG(VMBUS_DRV, "child device (%p) allocated - "
-		"type {%02x%02x%02x%02x-%02x%02x-%02x%02x-"
-		"%02x%02x%02x%02x%02x%02x%02x%02x},"
-		"id {%02x%02x%02x%02x-%02x%02x-%02x%02x-"
-		"%02x%02x%02x%02x%02x%02x%02x%02x}",
-		&child_device_ctx->device,
-		type->data[3], type->data[2], type->data[1], type->data[0],
-		type->data[5], type->data[4], type->data[7], type->data[6],
-		type->data[8], type->data[9], type->data[10], type->data[11],
-		type->data[12], type->data[13], type->data[14], type->data[15],
-		instance->data[3], instance->data[2],
-		instance->data[1], instance->data[0],
-		instance->data[5], instance->data[4],
-		instance->data[7], instance->data[6],
-		instance->data[8], instance->data[9],
-		instance->data[10], instance->data[11],
-		instance->data[12], instance->data[13],
-		instance->data[14], instance->data[15]);
-
 	child_device_obj = &child_device_ctx->device_obj;
 	child_device_obj->channel = channel;
 	memcpy(&child_device_obj->dev_type, type, sizeof(struct hv_guid));
@@ -730,9 +691,6 @@ int vmbus_child_device_register(struct hv_device *root_device_obj,
 				to_vm_device(child_device_obj);
 	static atomic_t device_num = ATOMIC_INIT(0);
 
-	DPRINT_DBG(VMBUS_DRV, "child device (%p) registering",
-		   child_device_ctx);
-
 	/* Set the device name. Otherwise, device_register() will fail. */
 	dev_set_name(&child_device_ctx->device, "vmbus_0_%d",
 		     atomic_inc_return(&device_num));
@@ -752,11 +710,11 @@ int vmbus_child_device_register(struct hv_device *root_device_obj,
 	ret = child_device_ctx->probe_error;
 
 	if (ret)
-		DPRINT_ERR(VMBUS_DRV, "unable to register child device (%p)",
-			   &child_device_ctx->device);
+		pr_err("%s: %s Unable to register child device",
+		       VMBUS_MOD, __func__);
 	else
-		DPRINT_INFO(VMBUS_DRV, "child device (%p) registered",
-			    &child_device_ctx->device);
+		pr_info("%s: Child device (%s) registered", VMBUS_MOD,
+			dev_name(&child_device_ctx->device));
 
 	return ret;
 }
@@ -769,17 +727,14 @@ void vmbus_child_device_unregister(struct hv_device *device_obj)
 {
 	struct vm_device *device_ctx = to_vm_device(device_obj);
 
-	DPRINT_INFO(VMBUS_DRV, "unregistering child device (%p)",
-		    &device_ctx->device);
-
 	/*
 	 * Kick off the process of unregistering the device.
 	 * This will call vmbus_remove() and eventually vmbus_device_release()
 	 */
 	device_unregister(&device_ctx->device);
 
-	DPRINT_INFO(VMBUS_DRV, "child device (%p) unregistered",
-		    &device_ctx->device);
+	pr_info("%s: child device %s unregistered", VMBUS_MOD,
+		    dev_name(&device_ctx->device));
 }
 
 /*
@@ -794,21 +749,6 @@ static int vmbus_uevent(struct device *device, struct kobj_uevent_env *env)
 	struct vm_device *device_ctx = device_to_vm_device(device);
 	int ret;
 
-	DPRINT_INFO(VMBUS_DRV, "generating uevent - VMBUS_DEVICE_CLASS_GUID={"
-		    "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
-		    "%02x%02x%02x%02x%02x%02x%02x%02x}",
-		    device_ctx->class_id.data[3], device_ctx->class_id.data[2],
-		    device_ctx->class_id.data[1], device_ctx->class_id.data[0],
-		    device_ctx->class_id.data[5], device_ctx->class_id.data[4],
-		    device_ctx->class_id.data[7], device_ctx->class_id.data[6],
-		    device_ctx->class_id.data[8], device_ctx->class_id.data[9],
-		    device_ctx->class_id.data[10],
-		    device_ctx->class_id.data[11],
-		    device_ctx->class_id.data[12],
-		    device_ctx->class_id.data[13],
-		    device_ctx->class_id.data[14],
-		    device_ctx->class_id.data[15]);
-
 	ret = add_uevent_var(env, "VMBUS_DEVICE_CLASS_GUID={"
 			     "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
 			     "%02x%02x%02x%02x%02x%02x%02x%02x}",
@@ -877,10 +817,6 @@ static int vmbus_match(struct device *device, struct device_driver *driver)
 			(struct vmbus_driver_context *)driver_ctx;
 
 		device_ctx->device_obj.drv = &vmbus_drv_ctx->drv_obj;
-		DPRINT_INFO(VMBUS_DRV,
-			    "device object (%p) set to driver object (%p)",
-			    &device_ctx->device_obj,
-			    device_ctx->device_obj.drv);
 
 		match = 1;
 	}
@@ -922,18 +858,17 @@ static int vmbus_probe(struct device *child_device)
 	if (driver_ctx->probe) {
 		ret = device_ctx->probe_error = driver_ctx->probe(child_device);
 		if (ret != 0) {
-			DPRINT_ERR(VMBUS_DRV, "probe() failed for device %s "
-				   "(%p) on driver %s (%d)...",
-				   dev_name(child_device), child_device,
-				   child_device->driver->name, ret);
+			pr_err("%s: %s failed for device %s (%d)",
+			       VMBUS_MOD, __func__,
+			       dev_name(child_device), ret);
 
 			INIT_WORK(&device_ctx->probe_failed_work_item,
 				  vmbus_probe_failed_cb);
 			schedule_work(&device_ctx->probe_failed_work_item);
 		}
 	} else {
-		DPRINT_ERR(VMBUS_DRV, "probe() method not set for driver - %s",
-			   child_device->driver->name);
+		pr_err("%s: %s not set for driver - %s",
+		       VMBUS_MOD, __func__, dev_name(child_device));
 		ret = -1;
 	}
 	return ret;
@@ -966,9 +901,8 @@ static int vmbus_remove(struct device *child_device)
 		if (driver_ctx->remove) {
 			ret = driver_ctx->remove(child_device);
 		} else {
-			DPRINT_ERR(VMBUS_DRV,
-				   "remove() method not set for driver - %s",
-				   child_device->driver->name);
+			pr_err("%s: %s not set for driver - %s",
+			       VMBUS_MOD, __func__, dev_name(child_device));
 			ret = -1;
 		}
 	}
@@ -1086,10 +1020,8 @@ MODULE_DEVICE_TABLE(dmi, microsoft_hv_dmi_table);
 
 static int __init vmbus_init(void)
 {
-	DPRINT_INFO(VMBUS_DRV,
-		"Vmbus initializing.... current log level 0x%x (%x,%x)",
-		vmbus_loglevel, HIWORD(vmbus_loglevel), LOWORD(vmbus_loglevel));
-	/* Todo: it is used for loglevel, to be ported to new kernel. */
+	pr_info("%s: initializing Version %s. Supported Hyper-V Rev %d.",
+		VMBUS_MOD, HV_DRV_VERSION, VMBUS_REVISION_NUMBER);
 
 	if (!dmi_check_system(microsoft_hv_dmi_table))
 		return -ENODEV;
@@ -1100,7 +1032,6 @@ static int __init vmbus_init(void)
 static void __exit vmbus_exit(void)
 {
 	vmbus_bus_exit();
-	/* Todo: it is used for loglevel, to be ported to new kernel. */
 }
 
 /*
-- 
1.6.0.2

[PATCH 2/6] Staging: hv: hv.c Removed all DPRINT and debug - using pr_err now

[Hank Janssen]

This group of patches removes all DPRINT from hv_vmbus.ko.
It is divided in several patches due to size.

All DPRINT calls have been removed, and where needed have been
replaced with pr_XX native calls. Many debug DPRINT calls have
been removed outright.

The amount of clutter this driver prints has been
significantly reduced.

Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/staging/hv/hv.c |   88 +++++++++++-----------------------------------
 1 files changed, 21 insertions(+), 67 deletions(-)

diff --git a/drivers/staging/hv/hv.c b/drivers/staging/hv/hv.c
index 2d492ad..e3ce26d 100644
--- a/drivers/staging/hv/hv.c
+++ b/drivers/staging/hv/hv.c
@@ -80,20 +80,6 @@ static int query_hypervisor_info(void)
 	op = HVCPUID_VENDOR_MAXFUNCTION;
 	cpuid(op, &eax, &ebx, &ecx, &edx);
 
-	DPRINT_INFO(VMBUS, "Vendor ID: %c%c%c%c%c%c%c%c%c%c%c%c",
-		    (ebx & 0xFF),
-		    ((ebx >> 8) & 0xFF),
-		    ((ebx >> 16) & 0xFF),
-		    ((ebx >> 24) & 0xFF),
-		    (ecx & 0xFF),
-		    ((ecx >> 8) & 0xFF),
-		    ((ecx >> 16) & 0xFF),
-		    ((ecx >> 24) & 0xFF),
-		    (edx & 0xFF),
-		    ((edx >> 8) & 0xFF),
-		    ((edx >> 16) & 0xFF),
-		    ((edx >> 24) & 0xFF));
-
 	max_leaf = eax;
 	eax = 0;
 	ebx = 0;
@@ -102,12 +88,6 @@ static int query_hypervisor_info(void)
 	op = HVCPUID_INTERFACE;
 	cpuid(op, &eax, &ebx, &ecx, &edx);
 
-	DPRINT_INFO(VMBUS, "Interface ID: %c%c%c%c",
-		    (eax & 0xFF),
-		    ((eax >> 8) & 0xFF),
-		    ((eax >> 16) & 0xFF),
-		    ((eax >> 24) & 0xFF));
-
 	if (max_leaf >= HVCPUID_VERSION) {
 		eax = 0;
 		ebx = 0;
@@ -115,14 +95,17 @@ static int query_hypervisor_info(void)
 		edx = 0;
 		op = HVCPUID_VERSION;
 		cpuid(op, &eax, &ebx, &ecx, &edx);
-		DPRINT_INFO(VMBUS, "OS Build:%d-%d.%d-%d-%d.%d",\
-			    eax,
-			    ebx >> 16,
-			    ebx & 0xFFFF,
-			    ecx,
-			    edx >> 24,
-			    edx & 0xFFFFFF);
+
+		pr_info("%s: Hyper-V Host OS Build:%d-%d.%d-%d-%d.%d",
+			VMBUS_MOD,
+			eax,
+			ebx >> 16,
+			ebx & 0xFFFF,
+			ecx,
+			edx >> 24,
+			edx & 0xFFFFFF);
 	}
+
 	return max_leaf;
 }
 
@@ -137,20 +120,12 @@ static u64 do_hypercall(u64 control, void *input, void *output)
 	u64 output_address = (output) ? virt_to_phys(output) : 0;
 	volatile void *hypercall_page = hv_context.hypercall_page;
 
-	DPRINT_DBG(VMBUS, "Hypercall <control %llx input phys %llx virt %p "
-		   "output phys %llx virt %p hypercall %p>",
-		   control, input_address, input,
-		   output_address, output, hypercall_page);
-
 	__asm__ __volatile__("mov %0, %%r8" : : "r" (output_address) : "r8");
 	__asm__ __volatile__("call *%3" : "=a" (hv_status) :
 			     "c" (control), "d" (input_address),
 			     "m" (hypercall_page));
 
-	DPRINT_DBG(VMBUS, "Hypercall <return %llx>",  hv_status);
-
 	return hv_status;
-
 #else
 
 	u32 control_hi = control >> 32;
@@ -165,18 +140,12 @@ static u64 do_hypercall(u64 control, void *input, void *output)
 	u32 output_address_lo = output_address & 0xFFFFFFFF;
 	volatile void *hypercall_page = hv_context.hypercall_page;
 
-	DPRINT_DBG(VMBUS, "Hypercall <control %llx input %p output %p>",
-		   control, input, output);
-
 	__asm__ __volatile__ ("call *%8" : "=d"(hv_status_hi),
 			      "=a"(hv_status_lo) : "d" (control_hi),
 			      "a" (control_lo), "b" (input_address_hi),
 			      "c" (input_address_lo), "D"(output_address_hi),
 			      "S"(output_address_lo), "m" (hypercall_page));
 
-	DPRINT_DBG(VMBUS, "Hypercall <return %llx>",
-		   hv_status_lo | ((u64)hv_status_hi << 32));
-
 	return hv_status_lo | ((u64)hv_status_hi << 32);
 #endif /* !x86_64 */
 }
@@ -198,13 +167,10 @@ int hv_init(void)
 	       sizeof(void *) * MAX_NUM_CPUS);
 
 	if (!query_hypervisor_presence()) {
-		DPRINT_ERR(VMBUS, "No Windows hypervisor detected!!");
+		pr_err("%s: %s No Hyper-V detected", VMBUS_MOD, __func__);
 		goto Cleanup;
 	}
 
-	DPRINT_INFO(VMBUS,
-		    "Windows hypervisor detected! Retrieving more info...");
-
 	max_leaf = query_hypervisor_info();
 	/* HvQueryHypervisorFeatures(maxLeaf); */
 
@@ -214,8 +180,8 @@ int hv_init(void)
 	rdmsrl(HV_X64_MSR_GUEST_OS_ID, hv_context.guestid);
 
 	if (hv_context.guestid != 0) {
-		DPRINT_ERR(VMBUS, "Unknown guest id (0x%llx)!!",
-				hv_context.guestid);
+		pr_err("%s: %s Unknown guest id (0x%llx)",
+		       VMBUS_MOD, __func__, hv_context.guestid);
 		goto Cleanup;
 	}
 
@@ -233,8 +199,8 @@ int hv_init(void)
 	virtaddr = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_EXEC);
 
 	if (!virtaddr) {
-		DPRINT_ERR(VMBUS,
-			   "unable to allocate hypercall page!!");
+		pr_err("%s: %s unable to allocate hypercall page",
+		       VMBUS_MOD, __func__);
 		goto Cleanup;
 	}
 
@@ -248,16 +214,13 @@ int hv_init(void)
 	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
 
 	if (!hypercall_msr.enable) {
-		DPRINT_ERR(VMBUS, "unable to set hypercall page!!");
+		pr_err("%s: %s Unable to set hypercall page",
+			VMBUS_MOD, __func__);
 		goto Cleanup;
 	}
 
 	hv_context.hypercall_page = virtaddr;
 
-	DPRINT_INFO(VMBUS, "Hypercall page VA=%p, PA=0x%0llx",
-		    hv_context.hypercall_page,
-		    (u64)hypercall_msr.guest_physical_address << PAGE_SHIFT);
-
 	/* Setup the global signal event param for the signal event hypercall */
 	hv_context.signal_event_buffer =
 			kmalloc(sizeof(struct hv_input_signal_event_buffer),
@@ -394,14 +357,12 @@ void hv_synic_init(void *irqarg)
 	/* Check the version */
 	rdmsrl(HV_X64_MSR_SVERSION, version);
 
-	DPRINT_INFO(VMBUS, "SynIC version: %llx", version);
-
 	hv_context.synic_message_page[cpu] =
 		(void *)get_zeroed_page(GFP_ATOMIC);
 
 	if (hv_context.synic_message_page[cpu] == NULL) {
-		DPRINT_ERR(VMBUS,
-			   "unable to allocate SYNIC message page!!");
+		pr_err("%s: %s Unable to allocate SYNIC message page",
+		       VMBUS_MOD, __func__);
 		goto Cleanup;
 	}
 
@@ -409,8 +370,8 @@ void hv_synic_init(void *irqarg)
 		(void *)get_zeroed_page(GFP_ATOMIC);
 
 	if (hv_context.synic_event_page[cpu] == NULL) {
-		DPRINT_ERR(VMBUS,
-			   "unable to allocate SYNIC event page!!");
+		pr_err("%s: %s Unable to allocate SYNIC event page",
+		       VMBUS_MOD, __func__);
 		goto Cleanup;
 	}
 
@@ -420,8 +381,6 @@ void hv_synic_init(void *irqarg)
 	simp.base_simp_gpa = virt_to_phys(hv_context.synic_message_page[cpu])
 		>> PAGE_SHIFT;
 
-	DPRINT_DBG(VMBUS, "HV_X64_MSR_SIMP msr set to: %llx", simp.as_uint64);
-
 	wrmsrl(HV_X64_MSR_SIMP, simp.as_uint64);
 
 	/* Setup the Synic's event page */
@@ -430,8 +389,6 @@ void hv_synic_init(void *irqarg)
 	siefp.base_siefp_gpa = virt_to_phys(hv_context.synic_event_page[cpu])
 		>> PAGE_SHIFT;
 
-	DPRINT_DBG(VMBUS, "HV_X64_MSR_SIEFP msr set to: %llx", siefp.as_uint64);
-
 	wrmsrl(HV_X64_MSR_SIEFP, siefp.as_uint64);
 
 	/* Setup the interception SINT. */
@@ -446,9 +403,6 @@ void hv_synic_init(void *irqarg)
 	shared_sint.masked = false;
 	shared_sint.auto_eoi = true;
 
-	DPRINT_DBG(VMBUS, "HV_X64_MSR_SINT1 msr set to: %llx",
-		   shared_sint.as_uint64);
-
 	wrmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
 
 	/* Enable the global synic bit */
-- 
1.6.0.2

[PATCH 3/6] Staging: hv: channel.c Removed debug DPRINTS use pr_err for errors

[Hank Janssen]

This group of patches removes all DPRINT from hv_vmbus.ko.
It is divided in several patches due to size.

All DPRINT calls have been removed, and where needed have been
replaced with pr_XX native calls. Many debug DPRINT calls have
been removed outright.

The amount of clutter this driver prints has been 
significantly reduced.

Several DPRINT calls remain in this file, they will be removed
in a subsequent patch. They are designed to print out a common
debug stream that will be implemented differently.

Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/staging/hv/channel.c |   71 ++++++-----------------------------------
 1 files changed, 11 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/hv/channel.c b/drivers/staging/hv/channel.c
index 775a52a..654e80c 100644
--- a/drivers/staging/hv/channel.c
+++ b/drivers/staging/hv/channel.c
@@ -213,9 +213,6 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 
 
 	/* Establish the gpadl for the ring buffer */
-	DPRINT_DBG(VMBUS, "Establishing ring buffer's gpadl for channel %p...",
-		   newchannel);
-
 	newchannel->ringbuffer_gpadlhandle = 0;
 
 	ret = vmbus_establish_gpadl(newchannel,
@@ -229,16 +226,6 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 		goto errorout;
 	}
 
-	DPRINT_DBG(VMBUS, "channel %p <relid %d gpadl 0x%x send ring %p "
-		   "size %d recv ring %p size %d, downstreamoffset %d>",
-		   newchannel, newchannel->offermsg.child_relid,
-		   newchannel->ringbuffer_gpadlhandle,
-		   newchannel->outbound.ring_buffer,
-		   newchannel->outbound.ring_size,
-		   newchannel->inbound.ring_buffer,
-		   newchannel->inbound.ring_size,
-		   send_ringbuffer_size);
-
 	/* Create and init the channel open message */
 	openInfo = kmalloc(sizeof(*openInfo) +
 			   sizeof(struct vmbus_channel_open_channel),
@@ -272,12 +259,11 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 		      &vmbus_connection.chn_msg_list);
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
-	DPRINT_DBG(VMBUS, "Sending channel open msg...");
-
 	ret = vmbus_post_msg(openMsg,
 			       sizeof(struct vmbus_channel_open_channel));
 	if (ret != 0) {
-		DPRINT_ERR(VMBUS, "unable to open channel - %d", ret);
+		pr_err("%s: %s Unable to open channel - %d",
+		       VMBUS_MOD, __func__, ret);
 		goto Cleanup;
 	}
 
@@ -291,10 +277,9 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 	}
 
 
-	if (openInfo->response.open_result.status == 0)
-		DPRINT_INFO(VMBUS, "channel <%p> open success!!", newchannel);
-	else
-		DPRINT_INFO(VMBUS, "channel <%p> open failed - %d!!",
+	if (openInfo->response.open_result.status)
+		pr_err("%s: %s Channel <%p> open failed - %d!!",
+		       VMBUS_MOD, __func__,
 			    newchannel, openInfo->response.open_result.status);
 
 Cleanup:
@@ -530,17 +515,13 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 		      &vmbus_connection.chn_msg_list);
 
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
-	DPRINT_DBG(VMBUS, "buffer %p, size %d msg cnt %d",
-		   kbuffer, size, msgcount);
-
-	DPRINT_DBG(VMBUS, "Sending GPADL Header - len %zd",
-		   msginfo->msgsize - sizeof(*msginfo));
 
 	msginfo->wait_condition = 0;
 	ret = vmbus_post_msg(gpadlmsg, msginfo->msgsize -
 			       sizeof(*msginfo));
 	if (ret != 0) {
-		DPRINT_ERR(VMBUS, "Unable to open channel - %d", ret);
+		pr_err("%s: %s Unable to open channel - %d",
+		       VMBUS_MOD, __func__, ret);
 		goto Cleanup;
 	}
 
@@ -556,10 +537,6 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 				CHANNELMSG_GPADL_BODY;
 			gpadl_body->gpadl = next_gpadl_handle;
 
-			DPRINT_DBG(VMBUS, "Sending GPADL Body - len %zd",
-				   submsginfo->msgsize -
-				   sizeof(*submsginfo));
-
 			dump_gpadl_body(gpadl_body, submsginfo->msgsize -
 				      sizeof(*submsginfo));
 			ret = vmbus_post_msg(gpadl_body,
@@ -577,12 +554,6 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 
 
 	/* At this point, we received the gpadl created msg */
-	DPRINT_DBG(VMBUS, "Received GPADL created "
-		   "(relid %d, status %d handle %x)",
-		   channel->offermsg.child_relid,
-		   msginfo->response.gpadl_created.creation_status,
-		   gpadlmsg->gpadl);
-
 	*gpadl_handle = gpadlmsg->gpadl;
 
 Cleanup:
@@ -730,9 +701,6 @@ int vmbus_sendpacket(struct vmbus_channel *channel, const void *buffer,
 	u64 aligned_data = 0;
 	int ret;
 
-	DPRINT_DBG(VMBUS, "channel %p buffer %p len %d",
-		   channel, buffer, bufferlen);
-
 	dump_vmbus_channel(channel);
 
 	/* ASSERT((packetLenAligned - packetLen) < sizeof(u64)); */
@@ -846,10 +814,6 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel,
 
 	dump_vmbus_channel(channel);
 
-	DPRINT_DBG(VMBUS, "data buffer - offset %u len %u pfn count %u",
-		multi_pagebuffer->offset,
-		multi_pagebuffer->len, pfncount);
-
 	if ((pfncount < 0) || (pfncount > MAX_MULTIPAGE_BUFFER_COUNT))
 		return -EINVAL;
 
@@ -927,7 +891,6 @@ int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer,
 	if (ret != 0) {
 		spin_unlock_irqrestore(&channel->inbound_lock, flags);
 
-		/* DPRINT_DBG(VMBUS, "nothing to read!!"); */
 		return 0;
 	}
 
@@ -937,18 +900,13 @@ int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer,
 	userlen = packetlen - (desc.offset8 << 3);
 	/* ASSERT(userLen > 0); */
 
-	DPRINT_DBG(VMBUS, "packet received on channel %p relid %d <type %d "
-		   "flag %d tid %llx pktlen %d datalen %d> ",
-		   channel, channel->offermsg.child_relid, desc.type,
-		   desc.flags, desc.trans_id, packetlen, userlen);
-
 	*buffer_actual_len = userlen;
 
 	if (userlen > bufferlen) {
 		spin_unlock_irqrestore(&channel->inbound_lock, flags);
 
-		DPRINT_ERR(VMBUS, "buffer too small - got %d needs %d",
-			   bufferlen, userlen);
+		pr_err("%s: %s Buffer too small - got %d need %d",
+		       VMBUS_MOD, __func__, bufferlen, userlen);
 		return -1;
 	}
 
@@ -987,7 +945,6 @@ int vmbus_recvpacket_raw(struct vmbus_channel *channel, void *buffer,
 	if (ret != 0) {
 		spin_unlock_irqrestore(&channel->inbound_lock, flags);
 
-		/* DPRINT_DBG(VMBUS, "nothing to read!!"); */
 		return 0;
 	}
 
@@ -996,18 +953,13 @@ int vmbus_recvpacket_raw(struct vmbus_channel *channel, void *buffer,
 	packetlen = desc.len8 << 3;
 	userlen = packetlen - (desc.offset8 << 3);
 
-	DPRINT_DBG(VMBUS, "packet received on channel %p relid %d <type %d "
-		   "flag %d tid %llx pktlen %d datalen %d> ",
-		   channel, channel->offermsg.child_relid, desc.type,
-		   desc.flags, desc.trans_id, packetlen, userlen);
-
 	*buffer_actual_len = packetlen;
 
 	if (packetlen > bufferlen) {
 		spin_unlock_irqrestore(&channel->inbound_lock, flags);
 
-		DPRINT_ERR(VMBUS, "buffer too small - needed %d bytes but "
-			   "got space for only %d bytes", packetlen, bufferlen);
+		pr_err("%s: %s Buffer too small - needed %d bytes got "
+		       "%d bytes", VMBUS_MOD, __func__, packetlen, bufferlen);
 		return -2;
 	}
 
@@ -1050,7 +1002,6 @@ void vmbus_ontimer(unsigned long data)
  */
 static void dump_vmbus_channel(struct vmbus_channel *channel)
 {
-	DPRINT_DBG(VMBUS, "Channel (%d)", channel->offermsg.child_relid);
 	dump_ring_info(&channel->outbound, "Outbound ");
 	dump_ring_info(&channel->inbound, "Inbound ");
 }
-- 
1.6.0.2

[PATCH 4/6] Staging: hv: channel_mgmt.c Removed DPRINT and implemented pr_XX

[Hank Janssen]

This group of patches removes all DPRINT from hv_vmbus.ko.
It is divided in several patches due to size.

All DPRINT calls have been removed, and where needed have been
replaced with pr_XX native calls. Many debug DPRINT calls have
been removed outright.

The amount of clutter this driver prints has been
significantly reduced.

Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/staging/hv/channel_mgmt.c |   73 +++++++------------------------------
 1 files changed, 14 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/hv/channel_mgmt.c b/drivers/staging/hv/channel_mgmt.c
index 0781c0e..752de54 100644
--- a/drivers/staging/hv/channel_mgmt.c
+++ b/drivers/staging/hv/channel_mgmt.c
@@ -290,9 +290,7 @@ static void release_channel(struct work_struct *work)
 						     struct vmbus_channel,
 						     work);
 
-	DPRINT_DBG(VMBUS, "releasing channel (%p)", channel);
 	destroy_workqueue(channel->controlwq);
-	DPRINT_DBG(VMBUS, "channel released (%p)", channel);
 
 	kfree(channel);
 }
@@ -384,8 +382,6 @@ static void vmbus_process_offer(struct work_struct *work)
 	spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
 
 	if (!fnew) {
-		DPRINT_DBG(VMBUS, "Ignoring duplicate offer for relid (%d)",
-			   newchannel->offermsg.child_relid);
 		free_channel(newchannel);
 		return;
 	}
@@ -400,9 +396,6 @@ static void vmbus_process_offer(struct work_struct *work)
 		&newchannel->offermsg.offer.if_instance,
 		newchannel);
 
-	DPRINT_DBG(VMBUS, "child device object allocated - %p",
-		   newchannel->device_obj);
-
 	/*
 	 * Add the new device to the bus. This will kick off device-driver
 	 * binding which eventually invokes the device driver's AddDevice()
@@ -410,9 +403,8 @@ static void vmbus_process_offer(struct work_struct *work)
 	 */
 	ret = vmbus_child_dev_add(newchannel->device_obj);
 	if (ret != 0) {
-		DPRINT_ERR(VMBUS,
-			   "unable to add child device object (relid %d)",
-			   newchannel->offermsg.child_relid);
+		pr_err("%s: %s Unable to add child device object (relid %d)",
+		       VMBUS_MOD, __func__, newchannel->offermsg.child_relid);
 
 		spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
 		list_del(&newchannel->listentry);
@@ -437,8 +429,8 @@ static void vmbus_process_offer(struct work_struct *work)
 						 hv_cb_utils[cnt].callback,
 						 newchannel) == 0) {
 				hv_cb_utils[cnt].channel = newchannel;
-				DPRINT_INFO(VMBUS, "%s",
-						hv_cb_utils[cnt].log_msg);
+				pr_info("%s: %s",
+					VMBUS_MOD, hv_cb_utils[cnt].log_msg);
 				count_hv_channel();
 			}
 		}
@@ -471,48 +463,20 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
 	}
 
 	if (!fsupported) {
-		DPRINT_DBG(VMBUS, "Ignoring channel offer notification for "
-			   "child relid %d", offer->child_relid);
 		return;
 	}
 
 	guidtype = &offer->offer.if_type;
 	guidinstance = &offer->offer.if_instance;
 
-	DPRINT_INFO(VMBUS, "Channel offer notification - "
-		    "child relid %d monitor id %d allocated %d, "
-		    "type {%02x%02x%02x%02x-%02x%02x-%02x%02x-"
-		    "%02x%02x%02x%02x%02x%02x%02x%02x} "
-		    "instance {%02x%02x%02x%02x-%02x%02x-%02x%02x-"
-		    "%02x%02x%02x%02x%02x%02x%02x%02x}",
-		    offer->child_relid, offer->monitorid,
-		    offer->monitor_allocated,
-		    guidtype->data[3], guidtype->data[2],
-		    guidtype->data[1], guidtype->data[0],
-		    guidtype->data[5], guidtype->data[4],
-		    guidtype->data[7], guidtype->data[6],
-		    guidtype->data[8], guidtype->data[9],
-		    guidtype->data[10], guidtype->data[11],
-		    guidtype->data[12], guidtype->data[13],
-		    guidtype->data[14], guidtype->data[15],
-		    guidinstance->data[3], guidinstance->data[2],
-		    guidinstance->data[1], guidinstance->data[0],
-		    guidinstance->data[5], guidinstance->data[4],
-		    guidinstance->data[7], guidinstance->data[6],
-		    guidinstance->data[8], guidinstance->data[9],
-		    guidinstance->data[10], guidinstance->data[11],
-		    guidinstance->data[12], guidinstance->data[13],
-		    guidinstance->data[14], guidinstance->data[15]);
-
 	/* Allocate the channel object and save this offer. */
 	newchannel = alloc_channel();
 	if (!newchannel) {
-		DPRINT_ERR(VMBUS, "unable to allocate channel object");
+		pr_err("%s: %s Unable to allocate channel object",
+		       VMBUS_MOD, __func__);
 		return;
 	}
 
-	DPRINT_DBG(VMBUS, "channel object allocated - %p", newchannel);
-
 	memcpy(&newchannel->offermsg, offer,
 	       sizeof(struct vmbus_channel_offer_channel));
 	newchannel->monitor_grp = (u8)offer->monitorid / 32;
@@ -536,8 +500,6 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 	rescind = (struct vmbus_channel_rescind_offer *)hdr;
 	channel = relid2channel(rescind->child_relid);
 	if (channel == NULL) {
-		DPRINT_DBG(VMBUS, "channel not found for relId %d",
-			   rescind->child_relid);
 		return;
 	}
 
@@ -573,7 +535,6 @@ static void vmbus_onopen_result(struct vmbus_channel_message_header *hdr)
 	unsigned long flags;
 
 	result = (struct vmbus_channel_open_result *)hdr;
-	DPRINT_DBG(VMBUS, "vmbus open result - %d", result->status);
 
 	/*
 	 * Find the open msg, copy the result and signal/unblock the wait event
@@ -618,8 +579,6 @@ static void vmbus_ongpadl_created(struct vmbus_channel_message_header *hdr)
 	unsigned long flags;
 
 	gpadlcreated = (struct vmbus_channel_gpadl_created *)hdr;
-	DPRINT_DBG(VMBUS, "vmbus gpadl created result - %d",
-		   gpadlcreated->creation_status);
 
 	/*
 	 * Find the establish msg, copy the result and signal/unblock the wait
@@ -770,12 +729,9 @@ void vmbus_onmessage(void *context)
 	hdr = (struct vmbus_channel_message_header *)msg->u.payload;
 	size = msg->header.payload_size;
 
-	DPRINT_DBG(VMBUS, "message type %d size %d", hdr->msgtype, size);
-
 	if (hdr->msgtype >= CHANNELMSG_COUNT) {
-		DPRINT_ERR(VMBUS,
-			   "Received invalid channel message type %d size %d",
-			   hdr->msgtype, size);
+		pr_err("%s: %s Received invalid channel msg type %d size %d",
+		       VMBUS_MOD, __func__, hdr->msgtype, size);
 		print_hex_dump_bytes("", DUMP_PREFIX_NONE,
 				     (unsigned char *)msg->u.payload, size);
 		return;
@@ -784,8 +740,8 @@ void vmbus_onmessage(void *context)
 	if (gChannelMessageTable[hdr->msgtype].messageHandler)
 		gChannelMessageTable[hdr->msgtype].messageHandler(hdr);
 	else
-		DPRINT_ERR(VMBUS, "Unhandled channel message type %d",
-			   hdr->msgtype);
+		pr_err("%s: %s Unhandled channel message type %d",
+		       VMBUS_MOD, __func__, hdr->msgtype);
 }
 
 /*
@@ -813,8 +769,8 @@ int vmbus_request_offers(void)
 	ret = vmbus_post_msg(msg,
 			       sizeof(struct vmbus_channel_message_header));
 	if (ret != 0) {
-		DPRINT_ERR(VMBUS, "Unable to request offers - %d", ret);
-
+		pr_err("%s: %s Unable to request offers - %d",
+		       VMBUS_MOD, __func__, ret);
 		goto cleanup;
 	}
 
@@ -854,9 +810,8 @@ void vmbus_release_unattached_channels(void)
 
 		if (!channel->device_obj->drv) {
 			list_del(&channel->listentry);
-			DPRINT_INFO(VMBUS,
-				    "Releasing unattached device object %p",
-				    channel->device_obj);
+			pr_info("%s: Releasing unattached dev object %p",
+				VMBUS_MOD, channel->device_obj);
 
 			vmbus_child_device_unregister(channel->device_obj);
 			free_channel(channel);
-- 
1.6.0.2

[PATCH 5/6] Staging: hv: ring_buffer.c Removed DPRINT replaced with pr_XX

[Hank Janssen]

This group of patches removes all DPRINT from hv_vmbus.ko.
It is divided in several patches due to size.

All DPRINT calls have been removed, and where needed have been
replaced with pr_XX native calls. Many debug DPRINT calls have
been removed outright.

The amount of clutter this driver prints has been
significantly reduced.

Several DPRINT calls remain in this file, they will be removed
in a subsequent patch. They are designed to print out a common
debug stream that will be implemented differently.

Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/staging/hv/ring_buffer.c |   25 ++++++++++---------------
 1 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/hv/ring_buffer.c b/drivers/staging/hv/ring_buffer.c
index 66688fb..95dd75c 100644
--- a/drivers/staging/hv/ring_buffer.c
+++ b/drivers/staging/hv/ring_buffer.c
@@ -372,19 +372,16 @@ int ringbuffer_write(struct hv_ring_buffer_info *outring_info,
 				&bytes_avail_toread,
 				&bytes_avail_towrite);
 
-	DPRINT_DBG(VMBUS, "Writing %u bytes...", totalbytes_towrite);
-
 	/* Dumpring_info(Outring_info, "BEFORE "); */
 
 	/* If there is only room for the packet, assume it is full. */
 	/* Otherwise, the next time around, we think the ring buffer */
 	/* is empty since the read index == write index */
 	if (bytes_avail_towrite <= totalbytes_towrite) {
-		DPRINT_DBG(VMBUS,
-			"No more space left on outbound ring buffer "
+		pr_debug("%s: %s No more space left on outbound ring buffer "
 			"(needed %u, avail %u)",
-			totalbytes_towrite,
-			bytes_avail_towrite);
+			 VMBUS_MOD, __func__, totalbytes_towrite,
+			 bytes_avail_towrite);
 
 		spin_unlock_irqrestore(&outring_info->ring_lock, flags);
 		return -1;
@@ -499,17 +496,13 @@ int ringbuffer_read(struct hv_ring_buffer_info *inring_info, void *buffer,
 				&bytes_avail_toread,
 				&bytes_avail_towrite);
 
-	DPRINT_DBG(VMBUS, "Reading %u bytes...", buflen);
-
 	/* Dumpring_info(Inring_info, "BEFORE "); */
 
 	/* Make sure there is something to read */
 	if (bytes_avail_toread < buflen) {
-		DPRINT_DBG(VMBUS,
-			"got callback but not enough to read "
-			"<avail to read %d read size %d>!!",
-			bytes_avail_toread,
-			buflen);
+		pr_debug("%s: %s got callback but not enough to read "
+			"<avail to read %d read size %d>",
+			 VMBUS_MOD, __func__, bytes_avail_toread, buflen);
 
 		spin_unlock_irqrestore(&inring_info->ring_lock, flags);
 
@@ -568,7 +561,8 @@ copyto_ringbuffer(
 
 	/* wrap-around detected! */
 	if (srclen > ring_buffer_size - start_write_offset) {
-		DPRINT_DBG(VMBUS, "wrap-around detected!");
+		pr_debug("%s: %s destination wrap-around detected!",
+			 VMBUS_MOD, __func__);
 
 		frag_len = ring_buffer_size - start_write_offset;
 		memcpy(ring_buffer + start_write_offset, src, frag_len);
@@ -607,7 +601,8 @@ copyfrom_ringbuffer(
 
 	/* wrap-around detected at the src */
 	if (destlen > ring_buffer_size - start_read_offset) {
-		DPRINT_DBG(VMBUS, "src wrap-around detected!");
+		pr_debug("%s: %s source wrap-around detected!",
+			 VMBUS_MOD, __func__);
 
 		frag_len = ring_buffer_size - start_read_offset;
 
-- 
1.6.0.2

[PATCH 6/6] Staging: hv: connection.c Removed DPRINT replaced with pr_XX

[Hank Janssen]

This group of patches removes all DPRINT from hv_vmbus.ko.
It is divided in several patches due to size.

All DPRINT calls have been removed, and where needed have been
replaced with pr_XX native calls. Many debug DPRINT calls have
been removed outright.

The amount of clutter this driver prints has been
significantly reduced.

Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/staging/hv/connection.c |   27 ++++++++++++---------------
 1 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/hv/connection.c b/drivers/staging/hv/connection.c
index f7df479..2e9c0b7 100644
--- a/drivers/staging/hv/connection.c
+++ b/drivers/staging/hv/connection.c
@@ -121,11 +121,6 @@ int vmbus_connect(void)
 
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
-	DPRINT_DBG(VMBUS, "Vmbus connection - interrupt pfn %llx, "
-		   "monitor1 pfn %llx,, monitor2 pfn %llx",
-		   msg->interrupt_page, msg->monitor_page1, msg->monitor_page2);
-
-	DPRINT_DBG(VMBUS, "Sending channel initiate msg...");
 	ret = vmbus_post_msg(msg,
 			       sizeof(struct vmbus_channel_initiate_contact));
 	if (ret != 0) {
@@ -156,13 +151,12 @@ int vmbus_connect(void)
 
 	/* Check if successful */
 	if (msginfo->response.version_response.version_supported) {
-		DPRINT_INFO(VMBUS, "Vmbus connected!!");
+		pr_info("%s: Connected to Hyper-V.", VMBUS_MOD);
 		vmbus_connection.conn_state = CONNECTED;
-
 	} else {
-		DPRINT_ERR(VMBUS, "Vmbus connection failed!!..."
-			   "current version (%d) not supported",
-			   VMBUS_REVISION_NUMBER);
+		pr_err("%s: %s Unable to connect, "
+		       "Version %d not supported by Hyper-V ",
+		       VMBUS_MOD, __func__, VMBUS_REVISION_NUMBER);
 		ret = -1;
 		goto Cleanup;
 	}
@@ -225,7 +219,7 @@ int vmbus_disconnect(void)
 
 	vmbus_connection.conn_state = DISCONNECTED;
 
-	DPRINT_INFO(VMBUS, "Vmbus disconnected!!");
+	pr_info("%s: Vmbus disconnected.", VMBUS_MOD);
 
 Cleanup:
 	kfree(msg);
@@ -278,7 +272,8 @@ static void process_chn_event(void *context)
 		 *			  (void*)channel);
 		 */
 	} else {
-		DPRINT_ERR(VMBUS, "channel not found for relid - %d.", relid);
+		pr_err("%s: %s channel not found for relid - %d.",
+		       VMBUS_MOD, __func__, relid);
 	}
 }
 
@@ -302,11 +297,13 @@ void vmbus_on_event(void)
 						(unsigned long *)
 						&recv_int_page[dword])) {
 						relid = (dword << 5) + bit;
-						DPRINT_DBG(VMBUS, "event detected for relid - %d", relid);
 
 						if (relid == 0) {
-							/* special case - vmbus channel protocol msg */
-							DPRINT_DBG(VMBUS, "invalid relid - %d", relid);
+							/*
+							 * special case -
+							 * vmbus channel
+							 * protocol msg
+							 */
 							continue;
 						} else {
 							/* QueueWorkItem(VmbusProcessEvent, (void*)relid); */
-- 
1.6.0.2

Re: [PATCH 1/6] Staging: hv: vmbus_drv.c Replaced DPRINT with native pr_XXX

[Joe Perches]

On Tue, 2011-02-22 at 15:32 -0800, Hank Janssen wrote:
> This group of patches removes all DPRINT from hv_vmbus.ko.
> It is divided in several patches due to size.
[]
> -		DPRINT_ERR(VMBUS_DRV,
> -			   "ERROR - Unable to register vmbus root device");
> +		pr_err("%s: %s ERROR Unable to register vmbus root device",
> +		       VMBUS_MOD, __func__);

All of the pr_<level> calls should probably have a terminating "\n"

Also, ff all the pr_<level>'s are using VMBUS_MOD,
then perhaps it would look better to add

#define pr_fmt(fmt) "%s: " fmt, VMBUS_MOD
or
#define pr_fmt(fmt) "%s:%s " fmt, VMBUS_MOD, __func__
(if you must)

and then use:

	pr_err("ERROR Unable to register vmbus root device\n");

RE: [PATCH 1/6] Staging: hv: vmbus_drv.c Replaced DPRINT with native pr_XXX

[Hank Janssen]

> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Tuesday, February 22, 2011 8:51 PM


> On Tue, 2011-02-22 at 15:32 -0800, Hank Janssen wrote:
> > This group of patches removes all DPRINT from hv_vmbus.ko.
> > It is divided in several patches due to size.
> []
> > -		DPRINT_ERR(VMBUS_DRV,
> > -			   "ERROR - Unable to register vmbus root device");
> > +		pr_err("%s: %s ERROR Unable to register vmbus root device",
> > +		       VMBUS_MOD, __func__);
> 
> All of the pr_<level> calls should probably have a terminating "\n"

I will correct them and resubmit.

> 
> Also, ff all the pr_<level>'s are using VMBUS_MOD,
> then perhaps it would look better to add
> 
> #define pr_fmt(fmt) "%s: " fmt, VMBUS_MOD
> or
> #define pr_fmt(fmt) "%s:%s " fmt, VMBUS_MOD, __func__
> (if you must)

I wrestled with that when I did the conversion, The reason I did not
Do that is when I check other drivers very few do it that way, most do
It the way I do it or actually hard code the module name to be printed.
And since the original objection was that DPRINT seems to implement it's
own logging I did not want to re-implement the pr_XXX calls with this layer
of indirection which almost looks like I am going down the path of DPRINT
again.

So what is the general consensus. The current way I implemented it seems
to be how other drivers use it to.

Hank.

RE: [PATCH 1/6] Staging: hv: vmbus_drv.c Replaced DPRINT with native pr_XXX

[Joe Perches]

On Wed, 2011-02-23 at 16:58 +0000, Hank Janssen wrote:
> > #define pr_fmt(fmt) "%s: " fmt, VMBUS_MOD
> > or
> > #define pr_fmt(fmt) "%s:%s " fmt, VMBUS_MOD, __func__
> > (if you must)
> I wrestled with that when I did the conversion, The reason I did not
> Do that is when I check other drivers very few do it that way, most do
> It the way I do it or actually hard code the module name to be printed.

My preference would have been to use the KBUILD_MODNAME that
almost every other driver/module uses.

> So what is the general consensus. The current way I implemented it seems
> to be how other drivers use it to.

Your code, your choices.

cheers, Joe

Re: [PATCH 1/6] Staging: hv: vmbus_drv.c Replaced DPRINT with native pr_XXX

[Dan Carpenter]

On Wed, Feb 23, 2011 at 04:58:10PM +0000, Hank Janssen wrote:
> > Also, ff all the pr_<level>'s are using VMBUS_MOD,
> > then perhaps it would look better to add
> > 
> > #define pr_fmt(fmt) "%s: " fmt, VMBUS_MOD
> > or
> > #define pr_fmt(fmt) "%s:%s " fmt, VMBUS_MOD, __func__
> > (if you must)
> 
> I wrestled with that when I did the conversion, The reason I did not
> Do that is when I check other drivers very few do it that way, most do
> It the way I do it or actually hard code the module name to be printed.
> And since the original objection was that DPRINT seems to implement it's
> own logging I did not want to re-implement the pr_XXX calls with this layer
> of indirection which almost looks like I am going down the path of DPRINT
> again.
> 
> So what is the general consensus. The current way I implemented it seems
> to be how other drivers use it to.

The way Joe suggested is best.

regards,
dan carpenter

RE: [PATCH 1/6] Staging: hv: vmbus_drv.c Replaced DPRINT with native pr_XXX

[Hank Janssen]

> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Wednesday, February 23, 2011 9:10 AM
> To: Hank Janssen
> On Wed, 2011-02-23 at 16:58 +0000, Hank Janssen wrote:
> > > #define pr_fmt(fmt) "%s: " fmt, VMBUS_MOD
> > > or
> > > #define pr_fmt(fmt) "%s:%s " fmt, VMBUS_MOD, __func__
> > > (if you must)
> > I wrestled with that when I did the conversion, The reason I did not
> > Do that is when I check other drivers very few do it that way, most
> do
> > It the way I do it or actually hard code the module name to be
> printed.
> 
> My preference would have been to use the KBUILD_MODNAME that
> almost every other driver/module uses.
> 
> > So what is the general consensus. The current way I implemented it
> seems
> > to be how other drivers use it to.
> 
> Your code, your choices.

Joe,

Hey I am easy! I am just not cheap! :)

Based on your feedback and Dan Carpenter's feedback I will implement
It the way you suggested it.

I should have the patch out later today.

Hank.

Re: [PATCH 1/6] Staging: hv: vmbus_drv.c Replaced DPRINT with native pr_XXX

[Greg KH]

On Tue, Feb 22, 2011 at 03:32:40PM -0800, Hank Janssen wrote:
> This group of patches removes all DPRINT from hv_vmbus.ko.
> It is divided in several patches due to size.

Why say this in the 1/6 patch?  It should be in the 0/6 introduction.

> 
> All DPRINT calls have been removed, and where needed have been
> replaced with pr_XX native calls. Many debug DPRINT calls have
> been removed outright.

I think a lot of these pr_XX calls can be switched to dev_XX calls
instead, right?

How about you break this up into a different series of patches to make
it more readable:
	- remove unneeded DPRINT calls
	- convert remaining DPRINT calls to pr_XX or dev_XX as needed

That would make it easier to review, as it is, it's quite difficult.

thanks,

greg k-h

Re: [PATCH 2/6] Staging: hv: hv.c Removed all DPRINT and debug - using pr_err now

[Greg KH]

On Tue, Feb 22, 2011 at 03:32:41PM -0800, Hank Janssen wrote:
> This group of patches removes all DPRINT from hv_vmbus.ko.
> It is divided in several patches due to size.
> 
> All DPRINT calls have been removed, and where needed have been
> replaced with pr_XX native calls. Many debug DPRINT calls have
> been removed outright.
> 
> The amount of clutter this driver prints has been
> significantly reduced.
> 
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> 
> ---
>  drivers/staging/hv/hv.c |   88 +++++++++++-----------------------------------
>  1 files changed, 21 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/staging/hv/hv.c b/drivers/staging/hv/hv.c
> index 2d492ad..e3ce26d 100644
> --- a/drivers/staging/hv/hv.c
> +++ b/drivers/staging/hv/hv.c
> @@ -80,20 +80,6 @@ static int query_hypervisor_info(void)
>  	op = HVCPUID_VENDOR_MAXFUNCTION;
>  	cpuid(op, &eax, &ebx, &ecx, &edx);
>  
> -	DPRINT_INFO(VMBUS, "Vendor ID: %c%c%c%c%c%c%c%c%c%c%c%c",
> -		    (ebx & 0xFF),
> -		    ((ebx >> 8) & 0xFF),
> -		    ((ebx >> 16) & 0xFF),
> -		    ((ebx >> 24) & 0xFF),
> -		    (ecx & 0xFF),
> -		    ((ecx >> 8) & 0xFF),
> -		    ((ecx >> 16) & 0xFF),
> -		    ((ecx >> 24) & 0xFF),
> -		    (edx & 0xFF),
> -		    ((edx >> 8) & 0xFF),
> -		    ((edx >> 16) & 0xFF),
> -		    ((edx >> 24) & 0xFF));
> -
>  	max_leaf = eax;
>  	eax = 0;
>  	ebx = 0;
> @@ -102,12 +88,6 @@ static int query_hypervisor_info(void)
>  	op = HVCPUID_INTERFACE;
>  	cpuid(op, &eax, &ebx, &ecx, &edx);
>  
> -	DPRINT_INFO(VMBUS, "Interface ID: %c%c%c%c",
> -		    (eax & 0xFF),
> -		    ((eax >> 8) & 0xFF),
> -		    ((eax >> 16) & 0xFF),
> -		    ((eax >> 24) & 0xFF));
> -
>  	if (max_leaf >= HVCPUID_VERSION) {
>  		eax = 0;
>  		ebx = 0;
> @@ -115,14 +95,17 @@ static int query_hypervisor_info(void)
>  		edx = 0;
>  		op = HVCPUID_VERSION;
>  		cpuid(op, &eax, &ebx, &ecx, &edx);
> -		DPRINT_INFO(VMBUS, "OS Build:%d-%d.%d-%d-%d.%d",\
> -			    eax,
> -			    ebx >> 16,
> -			    ebx & 0xFFFF,
> -			    ecx,
> -			    edx >> 24,
> -			    edx & 0xFFFFFF);
> +
> +		pr_info("%s: Hyper-V Host OS Build:%d-%d.%d-%d-%d.%d",
> +			VMBUS_MOD,
> +			eax,
> +			ebx >> 16,
> +			ebx & 0xFFFF,
> +			ecx,
> +			edx >> 24,
> +			edx & 0xFFFFFF);

Why did you keep this one?  Why is it needed?

>  	if (!query_hypervisor_presence()) {
> -		DPRINT_ERR(VMBUS, "No Windows hypervisor detected!!");
> +		pr_err("%s: %s No Hyper-V detected", VMBUS_MOD, __func__);

Why the __func__?  That should never be needed as it is trivial to see
what is happening, the user doesn't need to know the function name,
right?

Please remove them from all of these calls.

Oh, and you obviously didn't test these patches as your syslog would be
a mess if you did.  Which is NOT ok, and makes me grumpy:
	http://www.kroah.com/log/linux/maintainer-05.html

bah, I should just make a numbered list and just start saying: "This
patch fails point #4" or something like that, it would save me in
typing...

Please redo this entire series...

greg k-h

RE: [PATCH 2/6] Staging: hv: hv.c Removed all DPRINT and debug - using pr_err now

[Hank Janssen]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Wednesday, February 23, 2011 11:16 AM
> 
> On Tue, Feb 22, 2011 at 03:32:41PM -0800, Hank Janssen wrote:
> > This group of patches removes all DPRINT from hv_vmbus.ko.
> > It is divided in several patches due to size.
> >
> > All DPRINT calls have been removed, and where needed have been
> > replaced with pr_XX native calls. Many debug DPRINT calls have
> > been removed outright.
> >
> > The amount of clutter this driver prints has been
> > significantly reduced.
> >
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> >
> > ---
> >  drivers/staging/hv/hv.c |   88 +++++++++++--------------------------
> ---------
> >  1 files changed, 21 insertions(+), 67 deletions(-)

> > -		DPRINT_INFO(VMBUS, "OS Build:%d-%d.%d-%d-%d.%d",\
> > -			    eax,
> > -			    ebx >> 16,
> > -			    ebx & 0xFFFF,
> > -			    ecx,
> > -			    edx >> 24,
> > -			    edx & 0xFFFFFF);
> > +
> > +		pr_info("%s: Hyper-V Host OS Build:%d-%d.%d-%d-%d.%d",
> > +			VMBUS_MOD,
> > +			eax,
> > +			ebx >> 16,
> > +			ebx & 0xFFFF,
> > +			ecx,
> > +			edx >> 24,
> > +			edx & 0xFFFFFF);
> 
> Why did you keep this one?  Why is it needed?

This tells me what version the host is running. I frequently ask customers if 
they are running on Host version X or Y. This tells me that with certainty 
what they are running. The number of times I got from a customer that
they are running X while I knew that would not be possible has been pretty
high.

> 
> >  	if (!query_hypervisor_presence()) {
> > -		DPRINT_ERR(VMBUS, "No Windows hypervisor detected!!");
> > +		pr_err("%s: %s No Hyper-V detected", VMBUS_MOD, __func__);
> 
> Why the __func__?  That should never be needed as it is trivial to see
> what is happening, the user doesn't need to know the function name,
> right?
> 
> Please remove them from all of these calls.

The new patch will have these removed. When I checked other drivers 
in the drivers subdirectory the __func__ is used 15455 times most
of these are in print, debug or error lines. The __func__ in this
case only shows up if an error occurs. 

But as requested, I will remove them.

> 
> Oh, and you obviously didn't test these patches as your syslog would be
> a mess if you did.  Which is NOT ok, and makes me grumpy:
> 	http://www.kroah.com/log/linux/maintainer-05.html
> 
> bah, I should just make a numbered list and just start saying: "This
> patch fails point #4" or something like that, it would save me in
> typing...
> 

They where compile and run tested. And syslog was not a mess. What did
I mess up here? The amount of printouts now are a fraction of what they
where before.

Hank.

RE: [PATCH 1/6] Staging: hv: vmbus_drv.c Replaced DPRINT with native pr_XXX

[Hank Janssen]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Wednesday, February 23, 2011 11:12 AM
> To: Hank Janssen
> Why say this in the 1/6 patch?  It should be in the 0/6 introduction.
> 
> >
> > All DPRINT calls have been removed, and where needed have been
> > replaced with pr_XX native calls. Many debug DPRINT calls have
> > been removed outright.
> 
> I think a lot of these pr_XX calls can be switched to dev_XX calls
> instead, right?

Not in this case, this patch set is for vmbus only. The next patch
set will use netdev_* and dev* for netvsc, scsi and IDE.

> How about you break this up into a different series of patches to make
> it more readable:
> 	- remove unneeded DPRINT calls
> 	- convert remaining DPRINT calls to pr_XX or dev_XX as needed
> 
> That would make it easier to review, as it is, it's quite difficult.

Will do.

Hank.

Re: [PATCH 2/6] Staging: hv: hv.c Removed all DPRINT and debug - using pr_err now

[Greg KH]

On Wed, Feb 23, 2011 at 07:41:28PM +0000, Hank Janssen wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Wednesday, February 23, 2011 11:16 AM
> > 
> > On Tue, Feb 22, 2011 at 03:32:41PM -0800, Hank Janssen wrote:
> > > This group of patches removes all DPRINT from hv_vmbus.ko.
> > > It is divided in several patches due to size.
> > >
> > > All DPRINT calls have been removed, and where needed have been
> > > replaced with pr_XX native calls. Many debug DPRINT calls have
> > > been removed outright.
> > >
> > > The amount of clutter this driver prints has been
> > > significantly reduced.
> > >
> > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > >
> > > ---
> > >  drivers/staging/hv/hv.c |   88 +++++++++++--------------------------
> > ---------
> > >  1 files changed, 21 insertions(+), 67 deletions(-)
> 
> > > -		DPRINT_INFO(VMBUS, "OS Build:%d-%d.%d-%d-%d.%d",\
> > > -			    eax,
> > > -			    ebx >> 16,
> > > -			    ebx & 0xFFFF,
> > > -			    ecx,
> > > -			    edx >> 24,
> > > -			    edx & 0xFFFFFF);
> > > +
> > > +		pr_info("%s: Hyper-V Host OS Build:%d-%d.%d-%d-%d.%d",
> > > +			VMBUS_MOD,
> > > +			eax,
> > > +			ebx >> 16,
> > > +			ebx & 0xFFFF,
> > > +			ecx,
> > > +			edx >> 24,
> > > +			edx & 0xFFFFFF);
> > 
> > Why did you keep this one?  Why is it needed?
> 
> This tells me what version the host is running. I frequently ask customers if 
> they are running on Host version X or Y. This tells me that with certainty 
> what they are running. The number of times I got from a customer that
> they are running X while I knew that would not be possible has been pretty
> high.

Ok, fair enough.

> > >  	if (!query_hypervisor_presence()) {
> > > -		DPRINT_ERR(VMBUS, "No Windows hypervisor detected!!");
> > > +		pr_err("%s: %s No Hyper-V detected", VMBUS_MOD, __func__);
> > 
> > Why the __func__?  That should never be needed as it is trivial to see
> > what is happening, the user doesn't need to know the function name,
> > right?
> > 
> > Please remove them from all of these calls.
> 
> The new patch will have these removed. When I checked other drivers 
> in the drivers subdirectory the __func__ is used 15455 times most
> of these are in print, debug or error lines. The __func__ in this
> case only shows up if an error occurs. 
> 
> But as requested, I will remove them.

Thanks.

> > Oh, and you obviously didn't test these patches as your syslog would be
> > a mess if you did.  Which is NOT ok, and makes me grumpy:
> > 	http://www.kroah.com/log/linux/maintainer-05.html
> > 
> > bah, I should just make a numbered list and just start saying: "This
> > patch fails point #4" or something like that, it would save me in
> > typing...
> > 
> 
> They where compile and run tested. And syslog was not a mess. What did
> I mess up here? The amount of printouts now are a fraction of what they
> where before.

You forgot to put '\n' at the end of all of your pr_XXX lines, so they
will be merged with the next one, messing up your syslog.  Joe also
pointed this problem out.

Take a look at your syslog to see what I am talking about...

thanks,

greg k-h

RE: [PATCH 2/6] Staging: hv: hv.c Removed all DPRINT and debug - using pr_err now

[Hank Janssen]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Wednesday, February 23, 2011 1:57 PM
> They where compile and run tested. And syslog was not a mess. What did
> > I mess up here? The amount of printouts now are a fraction of what
> > they where before.
> 
> You forgot to put '\n' at the end of all of your pr_XXX lines, so they will be
> merged with the next one, messing up your syslog.  Joe also pointed this
> problem out.
> 
> Take a look at your syslog to see what I am talking about...

I am trying to get some of these patches corrected and out today, but it might
be tomorrow. So far I have lost power for a minutes or so twice this afternoon.

It seems that the threat of snow is enough to lose power where I am in the PNW.

Sigh......

Hank.

Re: [PATCH 2/6] Staging: hv: hv.c Removed all DPRINT and debug - using pr_err now

[Greg KH]

On Wed, Feb 23, 2011 at 11:17:54PM +0000, Hank Janssen wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Wednesday, February 23, 2011 1:57 PM
> > They where compile and run tested. And syslog was not a mess. What did
> > > I mess up here? The amount of printouts now are a fraction of what
> > > they where before.
> > 
> > You forgot to put '\n' at the end of all of your pr_XXX lines, so they will be
> > merged with the next one, messing up your syslog.  Joe also pointed this
> > problem out.
> > 
> > Take a look at your syslog to see what I am talking about...
> 
> I am trying to get some of these patches corrected and out today, but it might
> be tomorrow. So far I have lost power for a minutes or so twice this afternoon.

No rush, I've flushed out all of my staging tree patches today, so it
will be a number of days before I revisit them again.

thanks,

greg k-h

RE: [PATCH 2/6] Staging: hv: hv.c Removed all DPRINT and debug - using pr_err now

[Joe Perches]

On Wed, 2011-02-23 at 23:17 +0000, Hank Janssen wrote:
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Wednesday, February 23, 2011 1:57 PM
> > They where compile and run tested. And syslog was not a mess. What did
> > > I mess up here? The amount of printouts now are a fraction of what
> > > they where before.
> > You forgot to put '\n' at the end of all of your pr_XXX lines, so they will be
> > merged with the next one, messing up your syslog.  Joe also pointed this
> > problem out.
> > Take a look at your syslog to see what I am talking about...

Greg, there probably isn't any problem with his syslog.

Running together of messages without terminating newlines
did used to happen though until commit
5fd29d6ccbc98884569d6f3105aeca70858b3e0f
("printk: clean up handling of log-levels and newlines")
changed behavior so that newlines are emitted if necessary
before every "<.>" loglevel but "<c>".

That means that adding trailing newlines to pr_<level>
calls aren't _really_ necessary unless the pr_<level>
is followed by a bare printk without KERN_<LEVEL>.

I still think it's better from a style perspective to keep
adding terminating newlines until most all of the printks
are converted to pr_<level>.

Re: [PATCH 2/6] Staging: hv: hv.c Removed all DPRINT and debug - using pr_err now

[Greg KH]

On Wed, Feb 23, 2011 at 04:57:29PM -0800, Joe Perches wrote:
> On Wed, 2011-02-23 at 23:17 +0000, Hank Janssen wrote:
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@kroah.com]
> > > Sent: Wednesday, February 23, 2011 1:57 PM
> > > They where compile and run tested. And syslog was not a mess. What did
> > > > I mess up here? The amount of printouts now are a fraction of what
> > > > they where before.
> > > You forgot to put '\n' at the end of all of your pr_XXX lines, so they will be
> > > merged with the next one, messing up your syslog.  Joe also pointed this
> > > problem out.
> > > Take a look at your syslog to see what I am talking about...
> 
> Greg, there probably isn't any problem with his syslog.
> 
> Running together of messages without terminating newlines
> did used to happen though until commit
> 5fd29d6ccbc98884569d6f3105aeca70858b3e0f
> ("printk: clean up handling of log-levels and newlines")
> changed behavior so that newlines are emitted if necessary
> before every "<.>" loglevel but "<c>".
> 
> That means that adding trailing newlines to pr_<level>
> calls aren't _really_ necessary unless the pr_<level>
> is followed by a bare printk without KERN_<LEVEL>.

Ah, I didn't realize that this had changed, my mistake, nevermind :)

> I still think it's better from a style perspective to keep
> adding terminating newlines until most all of the printks
> are converted to pr_<level>.

Yes, that would be good to have done.

thanks,

greg k-h