lkml.org 
[lkml]   [2018]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v2 1/4] x86/hyper-v: move synic/stimer control structures definitions to hyperv-tlfs.h
Date
From: Vitaly Kuznetsov <vkuznets@redhat.com> Tuesday, November 27, 2018 5:11 AM

> > I personally tend to prefer masks over bitfields, so I'd rather do the
> > consolidation in the opposite direction: use the definitions in
> > hyperv-tlfs.h and replace those unions/bitfields elsewhere. (I vaguely
> > remember posting such a patchset a couple of years ago but I lacked the
> > motivation to complete it).
>
> Are there any known advantages of using masks over bitfields or the
> resulting binary code is the same? Assuming there are no I'd personally
> prefer bitfields - not sure why but to me e.g.
> (stimer->config.enabled && !stimer->config.direct_mode)
> looks nicer than
> (stimer->config & HV_STIMER_ENABLED && !(stimer->config & HV_STIMER_DIRECT))
>
> + there's no need to do shifts, e.g.
>
> vector = stimer->config.apic_vector
>
> looks cleaner that
>
> vector = (stimer->config & HV_STIMER_APICVECTOR_MASK) >>
> HV_STIMER_APICVECTOR_SHIFT
>
> ... and we already use a few in hyperv-tlfs.h. I'm, however, flexible :-)
>
> K. Y., Michael, Haiyang, Stephen - would you prefer to get rid of all
> bitfields from hyperv-tlfs.h? If so I can work on a patchset. As to this
> series, my understanding is that Paolo already queued it so in any case
> this is going to be a separate effort (unless there are strong
> objections of course).
>

I prefer to keep the bit fields. I concur think it makes the code more
readable. Even if there is a modest performance benefit, at least
within a Linux guest most of the manipulation of the fields is during
initialization when performance doesn't matter. But I can't speak to
KVM's implementation of the hypervisor side.

Michael

\
 
 \ /
  Last update: 2018-11-27 16:54    [W:0.121 / U:0.460 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site