LinuxLists.cc - [PATCH 2/6] Staging: hv: hv.c Removed all DPRINT and debug

2011-02-22 23:24:13

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

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 <[email protected]>
Signed-off-by: Haiyang Zhang <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>

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

2011-02-22 23:24:15

by Hank Janssen

[permalink] [raw]

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

2011-02-22 23:24:14

by Hank Janssen

[permalink] [raw]

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

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 <[email protected]>
Signed-off-by: Haiyang Zhang <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>

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

2011-02-22 23:24:21

by Hank Janssen

[permalink] [raw]

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

2011-02-22 23:24:11

by Hank Janssen

[permalink] [raw]

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

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 <[email protected]>
Signed-off-by: Haiyang Zhang <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>

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

2011-02-23 19:16:45

by Greg KH

[permalink] [raw]

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

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 <[email protected]>
> Signed-off-by: Haiyang Zhang <[email protected]>
> Signed-off-by: K. Y. Srinivasan <[email protected]>
>
> ---
> 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

2011-02-23 19:41:31

by Hank Janssen

[permalink] [raw]

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

> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> 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 <[email protected]>
> > Signed-off-by: Haiyang Zhang <[email protected]>
> > Signed-off-by: K. Y. Srinivasan <[email protected]>
> >
> > ---
> > 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.

2011-02-23 21:57:14

by Greg KH

[permalink] [raw]

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

On Wed, Feb 23, 2011 at 07:41:28PM +0000, Hank Janssen wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > 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 <[email protected]>
> > > Signed-off-by: Haiyang Zhang <[email protected]>
> > > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > >
> > > ---
> > > 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

2011-02-23 23:17:56

by Hank Janssen

[permalink] [raw]

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

> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> 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.

2011-02-23 23:55:08

by Greg KH

[permalink] [raw]

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

On Wed, Feb 23, 2011 at 11:17:54PM +0000, Hank Janssen wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > 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

2011-02-24 00:57:32

by Joe Perches

[permalink] [raw]

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

On Wed, 2011-02-23 at 23:17 +0000, Hank Janssen wrote:
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > 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>.

2011-02-24 01:28:30

by Greg KH

[permalink] [raw]

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

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:[email protected]]
> > > 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