| Message ID | 20161220155602.6298-4-rkagan@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 20, Roman Kagan wrote:
Reverting commit 22356585712d ("staging: hv: use sync_bitops when
interacting with the hypervisor") is save because .......
> - sync_set_bit(channel->monitor_bit,
> + set_bit(channel->monitor_bit,
Olaf
On Wed, Dec 21, 2016 at 01:00:44PM +0100, Olaf Hering wrote: > On Tue, Dec 20, Roman Kagan wrote: > > Reverting commit 22356585712d ("staging: hv: use sync_bitops when > interacting with the hypervisor") is save because ....... > > > - sync_set_bit(channel->monitor_bit, > > + set_bit(channel->monitor_bit, It isn't indeed. I didn't realize there was a UP case where it made a difference, and failed to locate the commit where it changed. I'll drop this part, thanks. Roman. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Roman Kagan [mailto:rkagan@virtuozzo.com] > Sent: Tuesday, December 20, 2016 7:56 AM > To: Paolo Bonzini <pbonzini@redhat.com>; Radim Krčmář > <rkrcmar@redhat.com>; KY Srinivasan <kys@microsoft.com>; Vitaly > Kuznetsov <vkuznets@redhat.com> > Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar > <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>; x86@kernel.org; > Haiyang Zhang <haiyangz@microsoft.com>; kvm@vger.kernel.org; linux- > kernel@vger.kernel.org; devel@linuxdriverproject.org; Denis V . Lunev > <den@openvz.org>; Roman Kagan <rkagan@virtuozzo.com> > Subject: [PATCH 03/15] hyperv: use standard bitops > No commit log? > Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> > --- > drivers/hv/channel.c | 8 +++----- > drivers/hv/connection.c | 9 +++------ > 2 files changed, 6 insertions(+), 11 deletions(-) > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c > index 5fb4c6d..f9df275 100644 > --- a/drivers/hv/channel.c > +++ b/drivers/hv/channel.c > @@ -49,15 +49,13 @@ void vmbus_setevent(struct vmbus_channel > *channel) > */ > if ((channel->offermsg.monitor_allocated) && > (!channel->low_latency)) { > - /* Each u32 represents 32 channels */ > - sync_set_bit(channel->offermsg.child_relid & 31, > - (unsigned long *) vmbus_connection.send_int_page > + > - (channel->offermsg.child_relid >> 5)); > + set_bit(channel->offermsg.child_relid, > + (unsigned long > *)vmbus_connection.send_int_page); > What is the rationale for dropping the sync variant? > /* Get the child to parent monitor page */ > monitorpage = vmbus_connection.monitor_pages[1]; > > - sync_set_bit(channel->monitor_bit, > + set_bit(channel->monitor_bit, > (unsigned long *)&monitorpage->trigger_group > [channel->monitor_grp].pending); > Same comment as before. > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c > index aaa2103..139b33e 100644 > --- a/drivers/hv/connection.c > +++ b/drivers/hv/connection.c > @@ -468,12 +468,9 @@ void vmbus_set_event(struct vmbus_channel > *channel) > { > u32 child_relid = channel->offermsg.child_relid; > > - if (!channel->is_dedicated_interrupt) { > - /* Each u32 represents 32 channels */ > - sync_set_bit(child_relid & 31, > - (unsigned long *)vmbus_connection.send_int_page > + > - (child_relid >> 5)); > - } > + if (!channel->is_dedicated_interrupt) > + set_bit(child_relid, > + (unsigned long > *)vmbus_connection.send_int_page); > > hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, > NULL); > } > -- > 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21/12/2016 14:23, Roman Kagan wrote: > On Wed, Dec 21, 2016 at 01:00:44PM +0100, Olaf Hering wrote: >> On Tue, Dec 20, Roman Kagan wrote: >> >> Reverting commit 22356585712d ("staging: hv: use sync_bitops when >> interacting with the hypervisor") is save because ....... >> >>> - sync_set_bit(channel->monitor_bit, >>> + set_bit(channel->monitor_bit, > > It isn't indeed. I didn't realize there was a UP case where it made a > difference, and failed to locate the commit where it changed. > > I'll drop this part, thanks. Perhaps the sync_bitops should be renamed to virt_bitops. This would match virt_* memory barriers and would make their usage much more obvious. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 5fb4c6d..f9df275 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -49,15 +49,13 @@ void vmbus_setevent(struct vmbus_channel *channel) */ if ((channel->offermsg.monitor_allocated) && (!channel->low_latency)) { - /* Each u32 represents 32 channels */ - sync_set_bit(channel->offermsg.child_relid & 31, - (unsigned long *) vmbus_connection.send_int_page + - (channel->offermsg.child_relid >> 5)); + set_bit(channel->offermsg.child_relid, + (unsigned long *)vmbus_connection.send_int_page); /* Get the child to parent monitor page */ monitorpage = vmbus_connection.monitor_pages[1]; - sync_set_bit(channel->monitor_bit, + set_bit(channel->monitor_bit, (unsigned long *)&monitorpage->trigger_group [channel->monitor_grp].pending); diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index aaa2103..139b33e 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -468,12 +468,9 @@ void vmbus_set_event(struct vmbus_channel *channel) { u32 child_relid = channel->offermsg.child_relid; - if (!channel->is_dedicated_interrupt) { - /* Each u32 represents 32 channels */ - sync_set_bit(child_relid & 31, - (unsigned long *)vmbus_connection.send_int_page + - (child_relid >> 5)); - } + if (!channel->is_dedicated_interrupt) + set_bit(child_relid, + (unsigned long *)vmbus_connection.send_int_page); hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, NULL); }
Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- drivers/hv/channel.c | 8 +++----- drivers/hv/connection.c | 9 +++------ 2 files changed, 6 insertions(+), 11 deletions(-)