Skip to content

Commit

Permalink
Staging: hv: Use only one txf buffer per channel and kmalloc/GFP_KERN…
Browse files Browse the repository at this point in the history
…EL 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.

And properly clean up memory on failure.

Thanks to Evgeniy Polyakov <zbr@ioremap.net> for pointing this out.
And thanks to Jesper Juhl <jj@chaosbits.net> and Ky Srinivasan
<ksrinivasan@novell.com> for suggesting a better implementation of
my original patch.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Reviewed-by: Jesper Juhl <jj@chaosbits.net>
Cc: Evgeniy Polyakov <zbr@ioremap.net>
Cc: Jesper Juhl <jj@chaosbits.net>
Cc: Ky Srinivasan <ksrinivasan@novell.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Hank Janssen authored and Greg Kroah-Hartman committed Dec 16, 2010
1 parent 244ba85 commit 45241e5
Showing 1 changed file with 47 additions and 40 deletions.
87 changes: 47 additions & 40 deletions drivers/staging/hv/hv_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@
#include "vmbus_api.h"
#include "utils.h"

static u8 *shut_txf_buf;
static u8 *time_txf_buf;
static u8 *hbeat_txf_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;

Expand All @@ -52,24 +54,23 @@ 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, shut_txf_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 *)&shut_txf_buf[
sizeof(struct vmbuspipe_hdr)];

if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
prep_negotiate_resp(icmsghdrp, negop, buf);
prep_negotiate_resp(icmsghdrp, negop, shut_txf_buf);
} else {
shutdown_msg = (struct shutdown_msg_data *)&buf[
sizeof(struct vmbuspipe_hdr) +
sizeof(struct icmsg_hdr)];
shutdown_msg =
(struct shutdown_msg_data *)&shut_txf_buf[
sizeof(struct vmbuspipe_hdr) +
sizeof(struct icmsg_hdr)];

switch (shutdown_msg->flags) {
case 0:
Expand All @@ -93,13 +94,11 @@ static void shutdown_onchannelcallback(void *context)
icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
| ICMSGHDRFLAG_RESPONSE;

vmbus_sendpacket(channel, buf,
vmbus_sendpacket(channel, shut_txf_buf,
recvlen, requestid,
VmbusPacketTypeDataInBand, 0);
}

kfree(buf);

if (execute_shutdown == true)
orderly_poweroff(false);
}
Expand Down Expand Up @@ -150,28 +149,25 @@ 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, time_txf_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 *)&time_txf_buf[
sizeof(struct vmbuspipe_hdr)];

if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
prep_negotiate_resp(icmsghdrp, NULL, buf);
prep_negotiate_resp(icmsghdrp, NULL, time_txf_buf);
} else {
timedatap = (struct ictimesync_data *)&buf[
timedatap = (struct ictimesync_data *)&time_txf_buf[
sizeof(struct vmbuspipe_hdr) +
sizeof(struct icmsg_hdr)];
adj_guesttime(timedatap->parenttime, timedatap->flags);
Expand All @@ -180,12 +176,10 @@ static void timesync_onchannelcallback(void *context)
icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
| ICMSGHDRFLAG_RESPONSE;

vmbus_sendpacket(channel, buf,
vmbus_sendpacket(channel, time_txf_buf,
recvlen, requestid,
VmbusPacketTypeDataInBand, 0);
}

kfree(buf);
}

/*
Expand All @@ -196,30 +190,28 @@ static void timesync_onchannelcallback(void *context)
static void heartbeat_onchannelcallback(void *context)
{
struct vmbus_channel *channel = context;
u8 *buf;
u32 buflen, recvlen;
u32 recvlen;
u64 requestid;
struct icmsg_hdr *icmsghdrp;
struct heartbeat_msg_data *heartbeat_msg;

buflen = PAGE_SIZE;
buf = kmalloc(buflen, GFP_ATOMIC);

vmbus_recvpacket(channel, buf, buflen, &recvlen, &requestid);
vmbus_recvpacket(channel, hbeat_txf_buf,
PAGE_SIZE, &recvlen, &requestid);

if (recvlen > 0) {
DPRINT_DBG(VMBUS, "heartbeat packet: len=%d, requestid=%lld",
recvlen, requestid);

icmsghdrp = (struct icmsg_hdr *)&buf[
icmsghdrp = (struct icmsg_hdr *)&hbeat_txf_buf[
sizeof(struct vmbuspipe_hdr)];

if (icmsghdrp->icmsgtype == ICMSGTYPE_NEGOTIATE) {
prep_negotiate_resp(icmsghdrp, NULL, buf);
prep_negotiate_resp(icmsghdrp, NULL, hbeat_txf_buf);
} else {
heartbeat_msg = (struct heartbeat_msg_data *)&buf[
sizeof(struct vmbuspipe_hdr) +
sizeof(struct icmsg_hdr)];
heartbeat_msg =
(struct heartbeat_msg_data *)&hbeat_txf_buf[
sizeof(struct vmbuspipe_hdr) +
sizeof(struct icmsg_hdr)];

DPRINT_DBG(VMBUS, "heartbeat seq = %lld",
heartbeat_msg->seq_num);
Expand All @@ -230,12 +222,10 @@ static void heartbeat_onchannelcallback(void *context)
icmsghdrp->icflags = ICMSGHDRFLAG_TRANSACTION
| ICMSGHDRFLAG_RESPONSE;

vmbus_sendpacket(channel, buf,
vmbus_sendpacket(channel, hbeat_txf_buf,
recvlen, requestid,
VmbusPacketTypeDataInBand, 0);
}

kfree(buf);
}

static const struct pci_device_id __initconst
Expand Down Expand Up @@ -268,6 +258,19 @@ static int __init init_hyperv_utils(void)
if (!dmi_check_system(hv_utils_dmi_table))
return -ENODEV;

shut_txf_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
time_txf_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
hbeat_txf_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);

if (!shut_txf_buf || !time_txf_buf || !hbeat_txf_buf) {
printk(KERN_INFO
"Unable to allocate memory for receive buffer\n");
kfree(shut_txf_buf);
kfree(time_txf_buf);
kfree(hbeat_txf_buf);
return -ENOMEM;
}

hv_cb_utils[HV_SHUTDOWN_MSG].channel->onchannel_callback =
&shutdown_onchannelcallback;
hv_cb_utils[HV_SHUTDOWN_MSG].callback = &shutdown_onchannelcallback;
Expand Down Expand Up @@ -298,6 +301,10 @@ static void exit_hyperv_utils(void)
hv_cb_utils[HV_HEARTBEAT_MSG].channel->onchannel_callback =
&chn_cb_negotiate;
hv_cb_utils[HV_HEARTBEAT_MSG].callback = &chn_cb_negotiate;

kfree(shut_txf_buf);
kfree(time_txf_buf);
kfree(hbeat_txf_buf);
}

module_init(init_hyperv_utils);
Expand Down

0 comments on commit 45241e5

Please sign in to comment.