* RE: [PATCH 1/1] Drivers: hv: hv_util: Avoid dynamic allocation in time synch
2016-09-16 16:01 ` [PATCH 1/1] Drivers: hv: hv_util: Avoid dynamic allocation in time synch kys
@ 2016-09-16 14:13 ` KY Srinivasan
2016-09-16 16:01 ` [PATCH 1/2] Drivers: hv: make VMBus bus ids persistent kys
2016-09-16 16:01 ` [PATCH 2/2] Drivers: hv: get rid of id in struct vmbus_channel kys
2 siblings, 0 replies; 6+ messages in thread
From: KY Srinivasan @ 2016-09-16 14:13 UTC (permalink / raw)
To: KY Srinivasan, gregkh, linux-kernel, devel, olaf, apw, vkuznets,
jasowang, leann.ogasawara
Cc: Vivek Yadav
> -----Original Message-----
> From: kys@exchange.microsoft.com [mailto:kys@exchange.microsoft.com]
> Sent: Friday, September 16, 2016 9:31 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com;
> leann.ogasawara@canonical.com
> Cc: Vivek Yadav <vyadav@microsoft.com>; KY Srinivasan
> <kys@microsoft.com>
> Subject: [PATCH 1/1] Drivers: hv: hv_util: Avoid dynamic allocation in time
> synch
>
> From: Vivek yadav <vyadav@microsoft.com>
>
> Under stress, we have seen allocation failure in time synch code. Avoid
> this dynamic allocation.
>
> Signed-off-by: Vivek Yadav <vyadav@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Greg,
Please ignore this patch, It is already committed and got sent because of a glitch.
Regards,
K. Y
> ---
> drivers/hv/hv_util.c | 39 ++++++++++++++++++++++++++++-----------
> 1 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 6286bdc..4aa3cb6 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -64,9 +64,14 @@ static struct hv_util_service util_shutdown = {
> .util_cb = shutdown_onchannelcallback,
> };
>
> +static int hv_timesync_init(struct hv_util_service *srv);
> +static void hv_timesync_deinit(void);
> +
> static void timesync_onchannelcallback(void *context);
> static struct hv_util_service util_timesynch = {
> .util_cb = timesync_onchannelcallback,
> + .util_init = hv_timesync_init,
> + .util_deinit = hv_timesync_deinit,
> };
>
> static void heartbeat_onchannelcallback(void *context);
> @@ -201,7 +206,6 @@ static void hv_set_host_time(struct work_struct
> *work)
> host_ts = ns_to_timespec(host_tns);
>
> do_settimeofday(&host_ts);
> - kfree(wrk);
> }
>
> /*
> @@ -217,22 +221,24 @@ static void hv_set_host_time(struct work_struct
> *work)
> * typically used as a hint to the guest. The guest is under no obligation
> * to discipline the clock.
> */
> +static struct adj_time_work wrk;
> static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 flags)
> {
> - struct adj_time_work *wrk;
>
> - wrk = kmalloc(sizeof(struct adj_time_work), GFP_ATOMIC);
> - if (wrk == NULL)
> + /*
> + * This check is safe since we are executing in the
> + * interrupt context and time synch messages arre always
> + * delivered on the same CPU.
> + */
> + if (work_pending(&wrk.work))
> return;
>
> - wrk->host_time = hosttime;
> - wrk->ref_time = reftime;
> - wrk->flags = flags;
> + wrk.host_time = hosttime;
> + wrk.ref_time = reftime;
> + wrk.flags = flags;
> if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) !=
> 0) {
> - INIT_WORK(&wrk->work, hv_set_host_time);
> - schedule_work(&wrk->work);
> - } else
> - kfree(wrk);
> + schedule_work(&wrk.work);
> + }
> }
>
> /*
> @@ -457,6 +463,17 @@ static struct hv_driver util_drv = {
> .remove = util_remove,
> };
>
> +static int hv_timesync_init(struct hv_util_service *srv)
> +{
> + INIT_WORK(&wrk.work, hv_set_host_time);
> + return 0;
> +}
> +
> +static void hv_timesync_deinit(void)
> +{
> + cancel_work_sync(&wrk.work);
> +}
> +
> static int __init init_hyperv_utils(void)
> {
> pr_info("Registering HyperV Utility Driver\n");
> --
> 1.7.4.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/2] Drivers: hv: vmbus: Use persistent IDs for vmbus device names
@ 2016-09-16 16:00 kys
2016-09-16 16:01 ` [PATCH 1/1] Drivers: hv: hv_util: Avoid dynamic allocation in time synch kys
0 siblings, 1 reply; 6+ messages in thread
From: kys @ 2016-09-16 16:00 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: K. Y. Srinivasan <kys@microsoft.com>
Use IDs that persist across reboots as vmbus device IDs.
Vitaly Kuznetsov (2):
Drivers: hv: make VMBus bus ids persistent
Drivers: hv: get rid of id in struct vmbus_channel
drivers/hv/channel_mgmt.c | 2 --
drivers/hv/vmbus_drv.c | 4 ++--
include/linux/hyperv.h | 3 ---
3 files changed, 2 insertions(+), 7 deletions(-)
--
1.7.4.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] Drivers: hv: hv_util: Avoid dynamic allocation in time synch
2016-09-16 16:00 [PATCH 0/2] Drivers: hv: vmbus: Use persistent IDs for vmbus device names kys
@ 2016-09-16 16:01 ` kys
2016-09-16 14:13 ` KY Srinivasan
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: kys @ 2016-09-16 16:01 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: Vivek yadav, K. Y. Srinivasan
From: Vivek yadav <vyadav@microsoft.com>
Under stress, we have seen allocation failure in time synch code. Avoid
this dynamic allocation.
Signed-off-by: Vivek Yadav <vyadav@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/hv_util.c | 39 ++++++++++++++++++++++++++++-----------
1 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 6286bdc..4aa3cb6 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -64,9 +64,14 @@ static struct hv_util_service util_shutdown = {
.util_cb = shutdown_onchannelcallback,
};
+static int hv_timesync_init(struct hv_util_service *srv);
+static void hv_timesync_deinit(void);
+
static void timesync_onchannelcallback(void *context);
static struct hv_util_service util_timesynch = {
.util_cb = timesync_onchannelcallback,
+ .util_init = hv_timesync_init,
+ .util_deinit = hv_timesync_deinit,
};
static void heartbeat_onchannelcallback(void *context);
@@ -201,7 +206,6 @@ static void hv_set_host_time(struct work_struct *work)
host_ts = ns_to_timespec(host_tns);
do_settimeofday(&host_ts);
- kfree(wrk);
}
/*
@@ -217,22 +221,24 @@ static void hv_set_host_time(struct work_struct *work)
* typically used as a hint to the guest. The guest is under no obligation
* to discipline the clock.
*/
+static struct adj_time_work wrk;
static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 flags)
{
- struct adj_time_work *wrk;
- wrk = kmalloc(sizeof(struct adj_time_work), GFP_ATOMIC);
- if (wrk == NULL)
+ /*
+ * This check is safe since we are executing in the
+ * interrupt context and time synch messages arre always
+ * delivered on the same CPU.
+ */
+ if (work_pending(&wrk.work))
return;
- wrk->host_time = hosttime;
- wrk->ref_time = reftime;
- wrk->flags = flags;
+ wrk.host_time = hosttime;
+ wrk.ref_time = reftime;
+ wrk.flags = flags;
if ((flags & (ICTIMESYNCFLAG_SYNC | ICTIMESYNCFLAG_SAMPLE)) != 0) {
- INIT_WORK(&wrk->work, hv_set_host_time);
- schedule_work(&wrk->work);
- } else
- kfree(wrk);
+ schedule_work(&wrk.work);
+ }
}
/*
@@ -457,6 +463,17 @@ static struct hv_driver util_drv = {
.remove = util_remove,
};
+static int hv_timesync_init(struct hv_util_service *srv)
+{
+ INIT_WORK(&wrk.work, hv_set_host_time);
+ return 0;
+}
+
+static void hv_timesync_deinit(void)
+{
+ cancel_work_sync(&wrk.work);
+}
+
static int __init init_hyperv_utils(void)
{
pr_info("Registering HyperV Utility Driver\n");
--
1.7.4.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] Drivers: hv: make VMBus bus ids persistent
2016-09-16 16:01 ` [PATCH 1/1] Drivers: hv: hv_util: Avoid dynamic allocation in time synch kys
2016-09-16 14:13 ` KY Srinivasan
@ 2016-09-16 16:01 ` kys
2016-09-16 16:01 ` [PATCH 2/2] Drivers: hv: get rid of id in struct vmbus_channel kys
2 siblings, 0 replies; 6+ messages in thread
From: kys @ 2016-09-16 16:01 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Some tools use bus ids to identify devices and they count on the fact
that these ids are persistent across reboot. This may be not true for
VMBus as we use auto incremented counter from alloc_channel() as such
id. Switch to using if_instance from channel offer, this id is supposed
to be persistent.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/vmbus_drv.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 6cbe074..a259e18 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -961,8 +961,8 @@ int vmbus_device_register(struct hv_device *child_device_obj)
{
int ret = 0;
- dev_set_name(&child_device_obj->device, "vmbus_%d",
- child_device_obj->channel->id);
+ dev_set_name(&child_device_obj->device, "vmbus-%pUl",
+ child_device_obj->channel->offermsg.offer.if_instance.b);
child_device_obj->device.bus = &hv_bus;
child_device_obj->device.parent = &hv_acpi_dev->dev;
--
1.7.4.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] Drivers: hv: get rid of id in struct vmbus_channel
2016-09-16 16:01 ` [PATCH 1/1] Drivers: hv: hv_util: Avoid dynamic allocation in time synch kys
2016-09-16 14:13 ` KY Srinivasan
2016-09-16 16:01 ` [PATCH 1/2] Drivers: hv: make VMBus bus ids persistent kys
@ 2016-09-16 16:01 ` kys
2 siblings, 0 replies; 6+ messages in thread
From: kys @ 2016-09-16 16:01 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
The auto incremented counter is not being used anymore, get rid of it.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel_mgmt.c | 2 --
include/linux/hyperv.h | 3 ---
2 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 759ba4d..96a85cd 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -274,14 +274,12 @@ EXPORT_SYMBOL_GPL(vmbus_prep_negotiate_resp);
*/
static struct vmbus_channel *alloc_channel(void)
{
- static atomic_t chan_num = ATOMIC_INIT(0);
struct vmbus_channel *channel;
channel = kzalloc(sizeof(*channel), GFP_ATOMIC);
if (!channel)
return NULL;
- channel->id = atomic_inc_return(&chan_num);
channel->acquire_ring_lock = true;
spin_lock_init(&channel->inbound_lock);
spin_lock_init(&channel->lock);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 7d7cbff..cd184bd 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -706,9 +706,6 @@ struct vmbus_device {
};
struct vmbus_channel {
- /* Unique channel id */
- int id;
-
struct list_head listentry;
struct hv_device *device_obj;
--
1.7.4.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] Drivers: hv: get rid of id in struct vmbus_channel
2016-08-09 8:46 [PATCH 0/2] Drivers: hv: vmbus: make bus ids in sysfs persistent Vitaly Kuznetsov
@ 2016-08-09 8:46 ` Vitaly Kuznetsov
0 siblings, 0 replies; 6+ messages in thread
From: Vitaly Kuznetsov @ 2016-08-09 8:46 UTC (permalink / raw)
To: devel; +Cc: linux-kernel, Haiyang Zhang, K. Y. Srinivasan
The auto incremented counter is not being used anymore, get rid of it.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/hv/channel_mgmt.c | 2 --
include/linux/hyperv.h | 3 ---
2 files changed, 5 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index b6c1211..4b4a41d 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -251,14 +251,12 @@ EXPORT_SYMBOL_GPL(vmbus_prep_negotiate_resp);
*/
static struct vmbus_channel *alloc_channel(void)
{
- static atomic_t chan_num = ATOMIC_INIT(0);
struct vmbus_channel *channel;
channel = kzalloc(sizeof(*channel), GFP_ATOMIC);
if (!channel)
return NULL;
- channel->id = atomic_inc_return(&chan_num);
channel->acquire_ring_lock = true;
spin_lock_init(&channel->inbound_lock);
spin_lock_init(&channel->lock);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index b10954a..d9371a7 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -701,9 +701,6 @@ struct vmbus_device {
};
struct vmbus_channel {
- /* Unique channel id */
- int id;
-
struct list_head listentry;
struct hv_device *device_obj;
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-16 14:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-16 16:00 [PATCH 0/2] Drivers: hv: vmbus: Use persistent IDs for vmbus device names kys
2016-09-16 16:01 ` [PATCH 1/1] Drivers: hv: hv_util: Avoid dynamic allocation in time synch kys
2016-09-16 14:13 ` KY Srinivasan
2016-09-16 16:01 ` [PATCH 1/2] Drivers: hv: make VMBus bus ids persistent kys
2016-09-16 16:01 ` [PATCH 2/2] Drivers: hv: get rid of id in struct vmbus_channel kys
-- strict thread matches above, loose matches on Subject: below --
2016-08-09 8:46 [PATCH 0/2] Drivers: hv: vmbus: make bus ids in sysfs persistent Vitaly Kuznetsov
2016-08-09 8:46 ` [PATCH 2/2] Drivers: hv: get rid of id in struct vmbus_channel Vitaly Kuznetsov
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®