This patch adds : * A byte-based cpumask type(xenctl_cpumask) for setting vcpu-affinity as well as numa-node-affinity, etc in libxc. * Add common bitmap utils to libxc, which can be used both for xenctl_cpumask (and with small changes for xenctl_cpumap, if desired), so that we can do common operations on cpumask easily. As opposed to xenctl_cpumap, xenctl_cpumask is a static structure (just 4 bytes larger for 128 cpus), but keeps the interface/code cleaner. The domctl_interface version keeps the size of xenctl_cpumask consistent between xen and xen-tools. -dulloor _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Mar-22 07:30 UTC
Re: [Xen-devel][PATCH] libxc bitmap utils and vcpu-affinity
On 22/03/2010 03:33, "Dulloor" <dulloor@gmail.com> wrote:> This patch adds : > > * A byte-based cpumask type(xenctl_cpumask) for setting vcpu-affinity > as well as numa-node-affinity, etc in libxc. > > * Add common bitmap utils to libxc, which can be used both for > xenctl_cpumask (and with small changes for xenctl_cpumap, if desired), > so that we can do common operations on cpumask easily. > > As opposed to xenctl_cpumap, xenctl_cpumask is a static structure > (just 4 bytes larger for 128 cpus), but keeps the interface/code > cleaner. The domctl_interface version keeps the size of xenctl_cpumask > consistent between xen and xen-tools.I''m missing the motivation for this. It sounds less good than what we have already. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> I''m missing the motivation for this. It sounds less good than what we have > already.xenctl_cpumap is dynamic and could work for any size, but there are many reasons for adding xenctl_cpumask. Motivation for new data type and bitmap utils in libxc : - xenctl_cpumask is a simple and useful data type in xen-tools. We deal with cpumasks at several places and the need increases with numa (for instance, node selection with guest numa). - The bitmap utils in the patch could work with both xenctl_cpumap as well as xenctl_cpumask structure. So, that part is independent of the new structure. And, I can just submit that part alone, if we decide to stay with xenctl_cpumap. Motivation for using xenctl_cpumask in Xen interfaces : - xenctl_cpumap is just 4 bytes smaller than static xenctl_cpumask for 128 cpus (128 would be good for quite some time). However, the new data type cleans up the code considerably, as you can see from its use in vcpu_(set|get)affinity - no need to read physinfo, lock pages separately, etc. - Maintaining the size of xenctl_cpumask consistent between xen and xen-tools is simple. And, conversion between xenctl_cpumask and cpumask inside xen is simple too (not much different from before). - There are other interfaces where xenctl_cpumask could be used. For pv guest numa, I pass cpumasks in the start-info and the structure needs to be static. The size of the structure is kept consistent through a numa-interface version. -dulloor On Mon, Mar 22, 2010 at 3:30 AM, Keir Fraser <keir.fraser@eu.citrix.com> wrote:> On 22/03/2010 03:33, "Dulloor" <dulloor@gmail.com> wrote: > >> This patch adds : >> >> * A byte-based cpumask type(xenctl_cpumask) for setting vcpu-affinity >> as well as numa-node-affinity, etc in libxc. >> >> * Add common bitmap utils to libxc, which can be used both for >> xenctl_cpumask (and with small changes for xenctl_cpumap, if desired), >> so that we can do common operations on cpumask easily. >> >> As opposed to xenctl_cpumap, xenctl_cpumask is a static structure >> (just 4 bytes larger for 128 cpus), but keeps the interface/code >> cleaner. The domctl_interface version keeps the size of xenctl_cpumask >> consistent between xen and xen-tools. > > I''m missing the motivation for this. It sounds less good than what we have > already. > > -- Keir > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Mar-23 10:10 UTC
Re: [Xen-devel][PATCH] libxc bitmap utils and vcpu-affinity
>>> Dulloor <dulloor@gmail.com> 22.03.10 18:44 >>> >Motivation for using xenctl_cpumask in Xen interfaces : >- xenctl_cpumap is just 4 bytes smaller than static xenctl_cpumask for >128 cpus (128 would be good for quite some time). However, the newI don''t buy this (we''re already building for 256 CPUs, looking forward to further bump this in the not too distant future), and I''m generally opposed to introducing hard coded limits in a public interface. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Mar-23 11:05 UTC
Re: [Xen-devel][PATCH] libxc bitmap utils and vcpu-affinity
On 23/03/2010 10:10, "Jan Beulich" <JBeulich@novell.com> wrote:>>>> Dulloor <dulloor@gmail.com> 22.03.10 18:44 >>> >> Motivation for using xenctl_cpumask in Xen interfaces : >> - xenctl_cpumap is just 4 bytes smaller than static xenctl_cpumask for >> 128 cpus (128 would be good for quite some time). However, the new > > I don''t buy this (we''re already building for 256 CPUs, looking forward > to further bump this in the not too distant future), and I''m generally > opposed to introducing hard coded limits in a public interface.We should use xenctl_cpumask everywhere for specifying physical CPU bitmaps, even into guest NUMA interfaces if appropriate. I don''t really care if it is a bit harder to use than a static bitmap. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Fine, I agree with you both. Attached is a patch adding utils for xenctl_bitmap (to libxc) and using the same in vcpu_(get|set)affinity. For the guest-numa interface, I will see if I can use xenctl_cpumap. -dulloor On Tue, Mar 23, 2010 at 7:05 AM, Keir Fraser <keir.fraser@eu.citrix.com> wrote:> On 23/03/2010 10:10, "Jan Beulich" <JBeulich@novell.com> wrote: > >>>>> Dulloor <dulloor@gmail.com> 22.03.10 18:44 >>> >>> Motivation for using xenctl_cpumask in Xen interfaces : >>> - xenctl_cpumap is just 4 bytes smaller than static xenctl_cpumask for >>> 128 cpus (128 would be good for quite some time). However, the new >> >> I don''t buy this (we''re already building for 256 CPUs, looking forward >> to further bump this in the not too distant future), and I''m generally >> opposed to introducing hard coded limits in a public interface. > > We should use xenctl_cpumask everywhere for specifying physical CPU bitmaps, > even into guest NUMA interfaces if appropriate. I don''t really care if it is > a bit harder to use than a static bitmap. > > -- Keir > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
I meant utils for **xenctl_cpumap** On Tue, Mar 23, 2010 at 12:40 PM, Dulloor <dulloor@gmail.com> wrote:> Fine, I agree with you both. Attached is a patch adding utils for > xenctl_bitmap (to libxc) and using the same in vcpu_(get|set)affinity. > For the guest-numa interface, I will see if I can use xenctl_cpumap. > > -dulloor > > On Tue, Mar 23, 2010 at 7:05 AM, Keir Fraser <keir.fraser@eu.citrix.com> wrote: >> On 23/03/2010 10:10, "Jan Beulich" <JBeulich@novell.com> wrote: >> >>>>>> Dulloor <dulloor@gmail.com> 22.03.10 18:44 >>> >>>> Motivation for using xenctl_cpumask in Xen interfaces : >>>> - xenctl_cpumap is just 4 bytes smaller than static xenctl_cpumask for >>>> 128 cpus (128 would be good for quite some time). However, the new >>> >>> I don''t buy this (we''re already building for 256 CPUs, looking forward >>> to further bump this in the not too distant future), and I''m generally >>> opposed to introducing hard coded limits in a public interface. >> >> We should use xenctl_cpumask everywhere for specifying physical CPU bitmaps, >> even into guest NUMA interfaces if appropriate. I don''t really care if it is >> a bit harder to use than a static bitmap. >> >> -- Keir >> >> >> >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Please use this patch, in which length of bitmap is (physinfo.max_cpu_id+1), rather than (physinfo.nr_cpus). -dulloor On Tue, Mar 23, 2010 at 12:41 PM, Dulloor <dulloor@gmail.com> wrote:> I meant utils for **xenctl_cpumap** > > On Tue, Mar 23, 2010 at 12:40 PM, Dulloor <dulloor@gmail.com> wrote: >> Fine, I agree with you both. Attached is a patch adding utils for >> xenctl_bitmap (to libxc) and using the same in vcpu_(get|set)affinity. >> For the guest-numa interface, I will see if I can use xenctl_cpumap. >> >> -dulloor >> >> On Tue, Mar 23, 2010 at 7:05 AM, Keir Fraser <keir.fraser@eu.citrix.com> wrote: >>> On 23/03/2010 10:10, "Jan Beulich" <JBeulich@novell.com> wrote: >>> >>>>>>> Dulloor <dulloor@gmail.com> 22.03.10 18:44 >>> >>>>> Motivation for using xenctl_cpumask in Xen interfaces : >>>>> - xenctl_cpumap is just 4 bytes smaller than static xenctl_cpumask for >>>>> 128 cpus (128 would be good for quite some time). However, the new >>>> >>>> I don''t buy this (we''re already building for 256 CPUs, looking forward >>>> to further bump this in the not too distant future), and I''m generally >>>> opposed to introducing hard coded limits in a public interface. >>> >>> We should use xenctl_cpumask everywhere for specifying physical CPU bitmaps, >>> even into guest NUMA interfaces if appropriate. I don''t really care if it is >>> a bit harder to use than a static bitmap. >>> >>> -- Keir >>> >>> >>> >> >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dulloor
2010-Mar-30 14:42 UTC
Fwd: [Xen-devel][PATCH] libxc bitmap utils and vcpu-affinity
Resubmitting the patch. -dulloor ---------- Forwarded message ---------- From: Dulloor <dulloor@gmail.com> Date: Tue, Mar 23, 2010 at 12:55 PM Subject: Re: [Xen-devel][PATCH] libxc bitmap utils and vcpu-affinity To: Keir Fraser <keir.fraser@eu.citrix.com> Cc: Jan Beulich <JBeulich@novell.com>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com> Please use this patch, in which length of bitmap is (physinfo.max_cpu_id+1), rather than (physinfo.nr_cpus). -dulloor On Tue, Mar 23, 2010 at 12:41 PM, Dulloor <dulloor@gmail.com> wrote:> I meant utils for **xenctl_cpumap** > > On Tue, Mar 23, 2010 at 12:40 PM, Dulloor <dulloor@gmail.com> wrote: >> Fine, I agree with you both. Attached is a patch adding utils for >> xenctl_bitmap (to libxc) and using the same in vcpu_(get|set)affinity. >> For the guest-numa interface, I will see if I can use xenctl_cpumap. >> >> -dulloor >> >> On Tue, Mar 23, 2010 at 7:05 AM, Keir Fraser <keir.fraser@eu.citrix.com> wrote: >>> On 23/03/2010 10:10, "Jan Beulich" <JBeulich@novell.com> wrote: >>> >>>>>>> Dulloor <dulloor@gmail.com> 22.03.10 18:44 >>> >>>>> Motivation for using xenctl_cpumask in Xen interfaces : >>>>> - xenctl_cpumap is just 4 bytes smaller than static xenctl_cpumask for >>>>> 128 cpus (128 would be good for quite some time). However, the new >>>> >>>> I don''t buy this (we''re already building for 256 CPUs, looking forward >>>> to further bump this in the not too distant future), and I''m generally >>>> opposed to introducing hard coded limits in a public interface. >>> >>> We should use xenctl_cpumask everywhere for specifying physical CPU bitmaps, >>> even into guest NUMA interfaces if appropriate. I don''t really care if it is >>> a bit harder to use than a static bitmap. >>> >>> -- Keir >>> >>> >>> >> >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Mar-30 15:16 UTC
Re: [Xen-devel][PATCH] libxc bitmap utils and vcpu-affinity
No changeset comment. No signed-off-by line. It actually bloats the
libraries by a net 650 LOC (747 added, 87 deleted according to diffstat).
And below I append the very first function I read: it doesn''t inspire
confidence that the implementation is over-complicated/long and
unnecessarily handles 16-bit values. Why should I show your patch some love?
-- Keir
+static inline int __xc_ffs(uint8_t byte)
+{
+ int num = 0;
+
+ if ((byte & 0xff) == 0) {
+ num += 8;
+ byte >>= 8;
+ }
+ if ((byte & 0xf) == 0) {
+ num += 4;
+ byte >>= 4;
+ }
+ if ((byte & 0x3) == 0) {
+ num += 2;
+ byte >>= 2;
+ }
+ if ((byte & 0x1) == 0)
+ num += 1;
+ return num;
+}
On 30/03/2010 15:42, "Dulloor" <dulloor@gmail.com> wrote:
> Resubmitting the patch.
>
> -dulloor
>
> ---------- Forwarded message ----------
> From: Dulloor <dulloor@gmail.com>
> Date: Tue, Mar 23, 2010 at 12:55 PM
> Subject: Re: [Xen-devel][PATCH] libxc bitmap utils and vcpu-affinity
> To: Keir Fraser <keir.fraser@eu.citrix.com>
> Cc: Jan Beulich <JBeulich@novell.com>,
"xen-devel@lists.xensource.com"
> <xen-devel@lists.xensource.com>
>
>
> Please use this patch, in which length of bitmap is
> (physinfo.max_cpu_id+1), rather than (physinfo.nr_cpus).
>
> -dulloor
>
> On Tue, Mar 23, 2010 at 12:41 PM, Dulloor <dulloor@gmail.com> wrote:
>> I meant utils for **xenctl_cpumap**
>>
>> On Tue, Mar 23, 2010 at 12:40 PM, Dulloor <dulloor@gmail.com>
wrote:
>>> Fine, I agree with you both. Attached is a patch adding utils for
>>> xenctl_bitmap (to libxc) and using the same in
vcpu_(get|set)affinity.
>>> For the guest-numa interface, I will see if I can use
xenctl_cpumap.
>>>
>>> -dulloor
>>>
>>> On Tue, Mar 23, 2010 at 7:05 AM, Keir Fraser
<keir.fraser@eu.citrix.com>
>>> wrote:
>>>> On 23/03/2010 10:10, "Jan Beulich"
<JBeulich@novell.com> wrote:
>>>>
>>>>>>>> Dulloor <dulloor@gmail.com> 22.03.10
18:44 >>>
>>>>>> Motivation for using xenctl_cpumask in Xen interfaces :
>>>>>> - xenctl_cpumap is just 4 bytes smaller than static
xenctl_cpumask for
>>>>>> 128 cpus (128 would be good for quite some time).
However, the new
>>>>>
>>>>> I don''t buy this (we''re already building
for 256 CPUs, looking forward
>>>>> to further bump this in the not too distant future), and
I''m generally
>>>>> opposed to introducing hard coded limits in a public
interface.
>>>>
>>>> We should use xenctl_cpumask everywhere for specifying physical
CPU
>>>> bitmaps,
>>>> even into guest NUMA interfaces if appropriate. I
don''t really care if it
>>>> is
>>>> a bit harder to use than a static bitmap.
>>>>
>>>> -- Keir
>>>>
>>>>
>>>>
>>>
>>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
> No changeset comment. No signed-off-by line.Sorry, I forgot. Will do once we finalize on the patch.> It actually bloats the libraries by a net 650 LOC > (747 added, 87 deleted according to diffstat).In the patch, we have used the library only for vcpu get/set affinity. There are clearly other opportunities (right now and in future) to use most of the functions provided by the library, which will offset this. Also, this provides a cleaner/standard way of using the cpumap structure in libxc.> And below I append the very first function I read: it doesn''t inspire > confidence that the implementation is over-complicated/long and > unnecessarily handles 16-bit values.I guess you mean 8-bit values. The library works with byte-based bitmap structures, instead of "uint32_t or uint64_t" bitmap structures, so that we don''t need to convert when using the bitmap-library with xenctl_cpumap. Please let know if you would rather that we keep the bitmap utilities separate from xenctl_cpumap and provide for conversion functions. That would be a small change. The function that you quote is inspired from linux kernel implementation (as mentioned) and is a simple generic function. And I have tested the library thoroughly.> Why should I show your patch some love?We can work towards that ;) -dulloor On Tue, Mar 30, 2010 at 11:16 AM, Keir Fraser <keir.fraser@eu.citrix.com> wrote:> No changeset comment. No signed-off-by line. It actually bloats the > libraries by a net 650 LOC (747 added, 87 deleted according to diffstat). > And below I append the very first function I read: it doesn''t inspire > confidence that the implementation is over-complicated/long and > unnecessarily handles 16-bit values. Why should I show your patch some love? > > -- Keir > > +static inline int __xc_ffs(uint8_t byte) > +{ > + int num = 0; > + > + if ((byte & 0xff) == 0) { > + num += 8; > + byte >>= 8; > + } > + if ((byte & 0xf) == 0) { > + num += 4; > + byte >>= 4; > + } > + if ((byte & 0x3) == 0) { > + num += 2; > + byte >>= 2; > + } > + if ((byte & 0x1) == 0) > + num += 1; > + return num; > +} > > On 30/03/2010 15:42, "Dulloor" <dulloor@gmail.com> wrote: > >> Resubmitting the patch. >> >> -dulloor >> >> ---------- Forwarded message ---------- >> From: Dulloor <dulloor@gmail.com> >> Date: Tue, Mar 23, 2010 at 12:55 PM >> Subject: Re: [Xen-devel][PATCH] libxc bitmap utils and vcpu-affinity >> To: Keir Fraser <keir.fraser@eu.citrix.com> >> Cc: Jan Beulich <JBeulich@novell.com>, "xen-devel@lists.xensource.com" >> <xen-devel@lists.xensource.com> >> >> >> Please use this patch, in which length of bitmap is >> (physinfo.max_cpu_id+1), rather than (physinfo.nr_cpus). >> >> -dulloor >> >> On Tue, Mar 23, 2010 at 12:41 PM, Dulloor <dulloor@gmail.com> wrote: >>> I meant utils for **xenctl_cpumap** >>> >>> On Tue, Mar 23, 2010 at 12:40 PM, Dulloor <dulloor@gmail.com> wrote: >>>> Fine, I agree with you both. Attached is a patch adding utils for >>>> xenctl_bitmap (to libxc) and using the same in vcpu_(get|set)affinity. >>>> For the guest-numa interface, I will see if I can use xenctl_cpumap. >>>> >>>> -dulloor >>>> >>>> On Tue, Mar 23, 2010 at 7:05 AM, Keir Fraser <keir.fraser@eu.citrix.com> >>>> wrote: >>>>> On 23/03/2010 10:10, "Jan Beulich" <JBeulich@novell.com> wrote: >>>>> >>>>>>>>> Dulloor <dulloor@gmail.com> 22.03.10 18:44 >>> >>>>>>> Motivation for using xenctl_cpumask in Xen interfaces : >>>>>>> - xenctl_cpumap is just 4 bytes smaller than static xenctl_cpumask for >>>>>>> 128 cpus (128 would be good for quite some time). However, the new >>>>>> >>>>>> I don''t buy this (we''re already building for 256 CPUs, looking forward >>>>>> to further bump this in the not too distant future), and I''m generally >>>>>> opposed to introducing hard coded limits in a public interface. >>>>> >>>>> We should use xenctl_cpumask everywhere for specifying physical CPU >>>>> bitmaps, >>>>> even into guest NUMA interfaces if appropriate. I don''t really care if it >>>>> is >>>>> a bit harder to use than a static bitmap. >>>>> >>>>> -- Keir >>>>> >>>>> >>>>> >>>> >>> > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Mar-30 16:27 UTC
Re: [Xen-devel][PATCH] libxc bitmap utils and vcpu-affinity
On 30/03/2010 17:05, "Dulloor" <dulloor@gmail.com> wrote:>> It actually bloats the libraries by a net 650 LOC >> (747 added, 87 deleted according to diffstat). > In the patch, we have used the library only for vcpu get/set affinity. > There are clearly other opportunities (right now and in future) to use > most of the functions provided by the library, which will offset this. > Also, this provides a cleaner/standard way of using the cpumap > structure in libxc.Clearly it''s not a simplicity win right now as it net adds a lot of code. I''d rather see this as part of a patch series that actually uses it more substantially. And even then I''d bet that half of this patch could be removed as unused. If NUMA changes end up manipulating cpumaps in, say, a dozen places then I could see this approach being useful, instead of pointless abstraction (how it appears currently). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel