* [PATCH] drivers: hv: allocate synic structures before hv_synic_init()
@ 2013-06-19 3:28 Jason Wang
2013-06-19 13:05 ` KY Srinivasan
0 siblings, 1 reply; 2+ messages in thread
From: Jason Wang @ 2013-06-19 3:28 UTC (permalink / raw)
To: kys, haiyangz, devel, linux-kernel, gregkh; +Cc: Jason Wang
We currently allocate synic structures in hv_sync_init(), but there's no way for
the driver to know about the allocation failure and it may continue to use the
uninitialized pointers. Solve this by introducing helpers for allocating and
freeing and doing the allocation before the on_each_cpu() call in
vmbus_bus_init().
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/hv/hv.c | 85 ++++++++++++++++++++++++++++-----------------
drivers/hv/hyperv_vmbus.h | 4 ++
drivers/hv/vmbus_drv.c | 8 +++-
3 files changed, 63 insertions(+), 34 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 3f88681..0039373 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -265,6 +265,59 @@ u16 hv_signal_event(void *con_id)
return status;
}
+
+int hv_synic_alloc(void)
+{
+ size_t size = sizeof(struct tasklet_struct);
+ int cpu;
+
+ for_each_online_cpu(cpu) {
+ hv_context.event_dpc[cpu] = kmalloc(size, GFP_ATOMIC);
+ if (hv_context.event_dpc[cpu] == NULL) {
+ pr_err("Unable to allocate event dpc\n");
+ goto err;
+ }
+ tasklet_init(hv_context.event_dpc[cpu], vmbus_on_event, cpu);
+
+ hv_context.synic_message_page[cpu] =
+ (void *)get_zeroed_page(GFP_ATOMIC);
+
+ if (hv_context.synic_message_page[cpu] == NULL) {
+ pr_err("Unable to allocate SYNIC message page\n");
+ goto err;
+ }
+
+ hv_context.synic_event_page[cpu] =
+ (void *)get_zeroed_page(GFP_ATOMIC);
+
+ if (hv_context.synic_event_page[cpu] == NULL) {
+ pr_err("Unable to allocate SYNIC event page\n");
+ goto err;
+ }
+ }
+
+ return 0;
+err:
+ return -ENOMEM;
+}
+
+void hv_synic_free_cpu(int cpu)
+{
+ kfree(hv_context.event_dpc[cpu]);
+ if (hv_context.synic_message_page[cpu])
+ free_page((unsigned long)hv_context.synic_event_page[cpu]);
+ if (hv_context.synic_message_page[cpu])
+ free_page((unsigned long)hv_context.synic_message_page[cpu]);
+}
+
+void hv_synic_free(void)
+{
+ int cpu;
+
+ for_each_online_cpu(cpu)
+ hv_synic_free_cpu(cpu);
+}
+
/*
* hv_synic_init - Initialize the Synthethic Interrupt Controller.
*
@@ -289,30 +342,6 @@ void hv_synic_init(void *arg)
/* Check the version */
rdmsrl(HV_X64_MSR_SVERSION, version);
- hv_context.event_dpc[cpu] = kmalloc(sizeof(struct tasklet_struct),
- GFP_ATOMIC);
- if (hv_context.event_dpc[cpu] == NULL) {
- pr_err("Unable to allocate event dpc\n");
- goto cleanup;
- }
- tasklet_init(hv_context.event_dpc[cpu], vmbus_on_event, cpu);
-
- hv_context.synic_message_page[cpu] =
- (void *)get_zeroed_page(GFP_ATOMIC);
-
- if (hv_context.synic_message_page[cpu] == NULL) {
- pr_err("Unable to allocate SYNIC message page\n");
- goto cleanup;
- }
-
- hv_context.synic_event_page[cpu] =
- (void *)get_zeroed_page(GFP_ATOMIC);
-
- if (hv_context.synic_event_page[cpu] == NULL) {
- pr_err("Unable to allocate SYNIC event page\n");
- goto cleanup;
- }
-
/* Setup the Synic's message page */
rdmsrl(HV_X64_MSR_SIMP, simp.as_uint64);
simp.simp_enabled = 1;
@@ -355,14 +384,6 @@ void hv_synic_init(void *arg)
rdmsrl(HV_X64_MSR_VP_INDEX, vp_index);
hv_context.vp_index[cpu] = (u32)vp_index;
return;
-
-cleanup:
- if (hv_context.synic_event_page[cpu])
- free_page((unsigned long)hv_context.synic_event_page[cpu]);
-
- if (hv_context.synic_message_page[cpu])
- free_page((unsigned long)hv_context.synic_message_page[cpu]);
- return;
}
/*
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 12f2f9e..d84918f 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -527,6 +527,10 @@ extern int hv_post_message(union hv_connection_id connection_id,
extern u16 hv_signal_event(void *con_id);
+extern int hv_synic_alloc(void);
+
+extern void hv_synic_free(void);
+
extern void hv_synic_init(void *irqarg);
extern void hv_synic_cleanup(void *arg);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 4004e54..a2464bf 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -563,6 +563,9 @@ static int vmbus_bus_init(int irq)
*/
hv_register_vmbus_handler(irq, vmbus_isr);
+ ret = hv_synic_alloc();
+ if (ret)
+ goto err_alloc;
/*
* Initialize the per-cpu interrupt state and
* connect to the host.
@@ -570,13 +573,14 @@ static int vmbus_bus_init(int irq)
on_each_cpu(hv_synic_init, NULL, 1);
ret = vmbus_connect();
if (ret)
- goto err_irq;
+ goto err_alloc;
vmbus_request_offers();
return 0;
-err_irq:
+err_alloc:
+ hv_synic_free();
free_irq(irq, hv_acpi_dev);
err_unregister:
--
1.7.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* RE: [PATCH] drivers: hv: allocate synic structures before hv_synic_init()
2013-06-19 3:28 [PATCH] drivers: hv: allocate synic structures before hv_synic_init() Jason Wang
@ 2013-06-19 13:05 ` KY Srinivasan
0 siblings, 0 replies; 2+ messages in thread
From: KY Srinivasan @ 2013-06-19 13:05 UTC (permalink / raw)
To: Jason Wang, Haiyang Zhang, devel, linux-kernel, gregkh
> -----Original Message-----
> From: Jason Wang [mailto:jasowang@redhat.com]
> Sent: Tuesday, June 18, 2013 11:28 PM
> To: KY Srinivasan; Haiyang Zhang; devel@linuxdriverproject.org; linux-
> kernel@vger.kernel.org; gregkh@linuxfoundation.org
> Cc: Jason Wang
> Subject: [PATCH] drivers: hv: allocate synic structures before hv_synic_init()
>
> We currently allocate synic structures in hv_sync_init(), but there's no way for
> the driver to know about the allocation failure and it may continue to use the
> uninitialized pointers. Solve this by introducing helpers for allocating and
> freeing and doing the allocation before the on_each_cpu() call in
> vmbus_bus_init().
>
> Cc: K. Y. Srinivasan <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/hv/hv.c | 85 ++++++++++++++++++++++++++++-----------------
> drivers/hv/hyperv_vmbus.h | 4 ++
> drivers/hv/vmbus_drv.c | 8 +++-
> 3 files changed, 63 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 3f88681..0039373 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -265,6 +265,59 @@ u16 hv_signal_event(void *con_id)
> return status;
> }
>
> +
> +int hv_synic_alloc(void)
> +{
> + size_t size = sizeof(struct tasklet_struct);
> + int cpu;
> +
> + for_each_online_cpu(cpu) {
> + hv_context.event_dpc[cpu] = kmalloc(size, GFP_ATOMIC);
> + if (hv_context.event_dpc[cpu] == NULL) {
> + pr_err("Unable to allocate event dpc\n");
> + goto err;
> + }
> + tasklet_init(hv_context.event_dpc[cpu], vmbus_on_event, cpu);
> +
> + hv_context.synic_message_page[cpu] =
> + (void *)get_zeroed_page(GFP_ATOMIC);
> +
> + if (hv_context.synic_message_page[cpu] == NULL) {
> + pr_err("Unable to allocate SYNIC message page\n");
> + goto err;
> + }
> +
> + hv_context.synic_event_page[cpu] =
> + (void *)get_zeroed_page(GFP_ATOMIC);
> +
> + if (hv_context.synic_event_page[cpu] == NULL) {
> + pr_err("Unable to allocate SYNIC event page\n");
> + goto err;
> + }
> + }
> +
> + return 0;
> +err:
> + return -ENOMEM;
> +}
> +
> +void hv_synic_free_cpu(int cpu)
> +{
> + kfree(hv_context.event_dpc[cpu]);
> + if (hv_context.synic_message_page[cpu])
> + free_page((unsigned long)hv_context.synic_event_page[cpu]);
> + if (hv_context.synic_message_page[cpu])
> + free_page((unsigned
> long)hv_context.synic_message_page[cpu]);
> +}
> +
> +void hv_synic_free(void)
> +{
> + int cpu;
> +
> + for_each_online_cpu(cpu)
> + hv_synic_free_cpu(cpu);
> +}
> +
> /*
> * hv_synic_init - Initialize the Synthethic Interrupt Controller.
> *
> @@ -289,30 +342,6 @@ void hv_synic_init(void *arg)
> /* Check the version */
> rdmsrl(HV_X64_MSR_SVERSION, version);
>
> - hv_context.event_dpc[cpu] = kmalloc(sizeof(struct tasklet_struct),
> - GFP_ATOMIC);
> - if (hv_context.event_dpc[cpu] == NULL) {
> - pr_err("Unable to allocate event dpc\n");
> - goto cleanup;
> - }
> - tasklet_init(hv_context.event_dpc[cpu], vmbus_on_event, cpu);
> -
> - hv_context.synic_message_page[cpu] =
> - (void *)get_zeroed_page(GFP_ATOMIC);
> -
> - if (hv_context.synic_message_page[cpu] == NULL) {
> - pr_err("Unable to allocate SYNIC message page\n");
> - goto cleanup;
> - }
> -
> - hv_context.synic_event_page[cpu] =
> - (void *)get_zeroed_page(GFP_ATOMIC);
> -
> - if (hv_context.synic_event_page[cpu] == NULL) {
> - pr_err("Unable to allocate SYNIC event page\n");
> - goto cleanup;
> - }
> -
> /* Setup the Synic's message page */
> rdmsrl(HV_X64_MSR_SIMP, simp.as_uint64);
> simp.simp_enabled = 1;
> @@ -355,14 +384,6 @@ void hv_synic_init(void *arg)
> rdmsrl(HV_X64_MSR_VP_INDEX, vp_index);
> hv_context.vp_index[cpu] = (u32)vp_index;
> return;
> -
> -cleanup:
> - if (hv_context.synic_event_page[cpu])
> - free_page((unsigned long)hv_context.synic_event_page[cpu]);
> -
> - if (hv_context.synic_message_page[cpu])
> - free_page((unsigned
> long)hv_context.synic_message_page[cpu]);
> - return;
> }
>
> /*
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 12f2f9e..d84918f 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -527,6 +527,10 @@ extern int hv_post_message(union hv_connection_id
> connection_id,
>
> extern u16 hv_signal_event(void *con_id);
>
> +extern int hv_synic_alloc(void);
> +
> +extern void hv_synic_free(void);
> +
> extern void hv_synic_init(void *irqarg);
>
> extern void hv_synic_cleanup(void *arg);
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 4004e54..a2464bf 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -563,6 +563,9 @@ static int vmbus_bus_init(int irq)
> */
> hv_register_vmbus_handler(irq, vmbus_isr);
>
> + ret = hv_synic_alloc();
> + if (ret)
> + goto err_alloc;
> /*
> * Initialize the per-cpu interrupt state and
> * connect to the host.
> @@ -570,13 +573,14 @@ static int vmbus_bus_init(int irq)
> on_each_cpu(hv_synic_init, NULL, 1);
> ret = vmbus_connect();
> if (ret)
> - goto err_irq;
> + goto err_alloc;
>
> vmbus_request_offers();
>
> return 0;
>
> -err_irq:
> +err_alloc:
> + hv_synic_free();
> free_irq(irq, hv_acpi_dev);
>
> err_unregister:
> --
> 1.7.1
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-06-19 13:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-19 3:28 [PATCH] drivers: hv: allocate synic structures before hv_synic_init() Jason Wang
2013-06-19 13:05 ` KY Srinivasan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®