[PATCH 1/1] hv: Use only one receive buffer and kmalloc on initialize
Correct issue with not checking kmalloc return value.
This fix now only uses one receive buffer for all hv_utils
channels, and will do only one kmalloc on init and will return
with a -ENOMEM if kmalloc fails on initialize.
Thanks to Evgeniy Polyakov z...@ioremap.net for pointing this out.
And thanks to Jesper Juhl j...@chaosbits.net and Ky Srinivasan
ksriniva...@novell.com for suggesting a better implementation of
my original patch.
Signed-off-by: Haiyang Zhang haiya...@microsoft.com
Signed-off-by: Hank Janssen hjans...@microsoft.com
Cc:Evgeniy Polyakov z...@ioremap.net
Cc:Jesper Juhl j...@chaosbits.net
Cc:Ky Srinivasan ksriniva...@novell.com
---
drivers/staging/hv/hv_utils.c | 68 +++-
1 files changed, 32 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/hv/hv_utils.c b/drivers/staging/hv/hv_utils.c
index 53e1e29..4ed4ab8 100644
--- a/drivers/staging/hv/hv_utils.c
+++ b/drivers/staging/hv/hv_utils.c
@@ -38,12 +38,15 @@
#include vmbus_api.h
#include utils.h
+/*
+ * Buffer used to receive packets from Hyper-V
+ */
+static u8 *chan_buf;
static void shutdown_onchannelcallback(void *context)
{
struct vmbus_channel *channel = context;
- u8 *buf;
- u32 buflen, recvlen;
+ u32 recvlen;
u64 requestid;
u8 execute_shutdown = false;
@@ -52,22 +55,19 @@ static void shutdown_onchannelcallback(void *context)
struct icmsg_hdr *icmsghdrp;
struct icmsg_negotiate *negop = NULL;
- buflen = PAGE_SIZE;
- buf = kmalloc(buflen, GFP_ATOMIC);
-
- vmbus_recvpacket(channel, buf, buflen, recvlen, requestid);
+ vmbus_recvpacket(channel, chan_buf, PAGE_SIZE, recvlen, requestid);
if (recvlen 0) {
DPRINT_DBG(VMBUS, shutdown packet: len=%d, requestid=%lld,
recvlen, requestid);
- icmsghdrp = (struct icmsg_hdr *)buf[
+ icmsghdrp = (struct icmsg_hdr *)chan_buf[
sizeof(struct vmbuspipe_hdr)];
if (icmsghdrp-icmsgtype == ICMSGTYPE_NEGOTIATE) {
- prep_negotiate_resp(icmsghdrp, negop, buf);
+ prep_negotiate_resp(icmsghdrp, negop, chan_buf);
} else {
- shutdown_msg = (struct shutdown_msg_data *)buf[
+ shutdown_msg = (struct shutdown_msg_data *)chan_buf[
sizeof(struct vmbuspipe_hdr) +
sizeof(struct icmsg_hdr)];
@@ -93,13 +93,11 @@ static void shutdown_onchannelcallback(void *context)
icmsghdrp-icflags = ICMSGHDRFLAG_TRANSACTION
| ICMSGHDRFLAG_RESPONSE;
- vmbus_sendpacket(channel, buf,
+ vmbus_sendpacket(channel, chan_buf,
recvlen, requestid,
VmbusPacketTypeDataInBand, 0);
}
- kfree(buf);
-
if (execute_shutdown == true)
orderly_poweroff(false);
}
@@ -150,28 +148,24 @@ static inline void adj_guesttime(u64 hosttime, u8 flags)
static void timesync_onchannelcallback(void *context)
{
struct vmbus_channel *channel = context;
- u8 *buf;
- u32 buflen, recvlen;
+ u32 recvlen;
u64 requestid;
struct icmsg_hdr *icmsghdrp;
struct ictimesync_data *timedatap;
- buflen = PAGE_SIZE;
- buf = kmalloc(buflen, GFP_ATOMIC);
-
- vmbus_recvpacket(channel, buf, buflen, recvlen, requestid);
+ vmbus_recvpacket(channel, chan_buf, PAGE_SIZE, recvlen, requestid);
if (recvlen 0) {
DPRINT_DBG(VMBUS, timesync packet: recvlen=%d, requestid=%lld,
recvlen, requestid);
- icmsghdrp = (struct icmsg_hdr *)buf[
+ icmsghdrp = (struct icmsg_hdr *)chan_buf[
sizeof(struct vmbuspipe_hdr)];
if (icmsghdrp-icmsgtype == ICMSGTYPE_NEGOTIATE) {
- prep_negotiate_resp(icmsghdrp, NULL, buf);
+ prep_negotiate_resp(icmsghdrp, NULL, chan_buf);
} else {
- timedatap = (struct ictimesync_data *)buf[
+ timedatap = (struct ictimesync_data *)chan_buf[
sizeof(struct vmbuspipe_hdr) +
sizeof(struct icmsg_hdr)];
adj_guesttime(timedatap-parenttime, timedatap-flags);
@@ -180,12 +174,10 @@ static void timesync_onchannelcallback(void *context)
icmsghdrp-icflags = ICMSGHDRFLAG_TRANSACTION
| ICMSGHDRFLAG_RESPONSE;
- vmbus_sendpacket(channel, buf,
+ vmbus_sendpacket(channel, chan_buf,
recvlen, requestid,
VmbusPacketTypeDataInBand, 0);
}
-
- kfree(buf);
}
/*
Re: [PATCH 1/1] hv: Use only one receive buffer and kmalloc on initialize
On Mon, Dec 13, 2010 at 07:31:42PM +, Hank Janssen wrote: -Original Message- From: Greg KH [mailto:g...@kroah.com] Sent: Monday, December 13, 2010 10:35 AM --- drivers/staging/hv/hv_utils.c | 68 +++-- --- 1 files changed, 32 insertions(+), 36 deletions(-) diff --git a/drivers/staging/hv/hv_utils.c b/drivers/staging/hv/hv_utils.c index 53e1e29..4ed4ab8 100644 --- a/drivers/staging/hv/hv_utils.c +++ b/drivers/staging/hv/hv_utils.c @@ -38,12 +38,15 @@ #include vmbus_api.h #include utils.h +/* + * Buffer used to receive packets from Hyper-V */ static u8 +*chan_buf; One buffer is nicer, yes, but what's controlling access to this buffer? You use it in multiple functions, and what's to say those functions can't be called at the same time on different cpus? So, shouldn't you either have some locking for access to the buffer, or have a per-function buffer instead? And if you have a per-function buffer, again, you might need to control access to it as the functions could be called multiple times at the same time, right? The current versions of Hyper-V support interrupt handling on CPU0 only. I can make multiple buffers per channel, but because of Hyper-V implementation It does not really make a difference. Then put a big fat note in there saying this, and that it will have to change if the hyperv channel ever changes. Hm, how will you handle things if the hyperv core changes and an old kernel is running this code where things were assumed about the reentrancy happening here? thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 1/1] hv: Use only one receive buffer and kmalloc on initialize
-Original Message- From: Greg KH [mailto:g...@kroah.com] Sent: Monday, December 13, 2010 11:41 AM The current versions of Hyper-V support interrupt handling on CPU0 only. I can make multiple buffers per channel, but because of Hyper-V implementation It does not really make a difference. Then put a big fat note in there saying this, and that it will have to change if the hyperv channel ever changes. Hm, how will you handle things if the hyperv core changes and an old kernel is running this code where things were assumed about the reentrancy happening here? I will re-roll this patch to have a buffer per channel. It is a more elegant design Even though Hyper-V behavior is not changing in Win8 for this. Hank. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/1] hv: Use only one receive buffer and kmalloc on initialize
On Mon, Dec 13, 2010 at 08:06:20PM +, Hank Janssen wrote: -Original Message- From: Greg KH [mailto:g...@kroah.com] Sent: Monday, December 13, 2010 11:41 AM The current versions of Hyper-V support interrupt handling on CPU0 only. I can make multiple buffers per channel, but because of Hyper-V implementation It does not really make a difference. Then put a big fat note in there saying this, and that it will have to change if the hyperv channel ever changes. Hm, how will you handle things if the hyperv core changes and an old kernel is running this code where things were assumed about the reentrancy happening here? I will re-roll this patch to have a buffer per channel. It is a more elegant design Even though Hyper-V behavior is not changing in Win8 for this. Win8, fine, but what about Win9? :) thanks for changing it. greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
