WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [xen-devel][vNUMA v2][PATCH 3/8] Basic cpumap utilities

To: Andre Przywara <andre.przywara@xxxxxxx>
Subject: Re: [xen-devel][vNUMA v2][PATCH 3/8] Basic cpumap utilities
From: Dulloor <dulloor@xxxxxxxxx>
Date: Fri, 13 Aug 2010 21:38:30 -0700
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 13 Aug 2010 21:39:15 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=lreKY6MF8+0+DRXbtJBu1CyETXIjJG9BHt0+9JnxOLE=; b=cjHWdfSrFtGMXuDS0onRR4flRJlAPIKhW0QcGzLGtGp2/jciGvsEd6TuPs5XAdTJKE 7GtGrsSNX3eHpJgT/+H1mlpkmbiCZYwaw++XSh/LFE5o1nm5pYFg74wrCk7nDc9vDGX3 hmtuqzdeXjYMd+2BazynzUwVkfmsV5cE1w2jk=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=MoAip6q7UkbIP9WZdMCP+0Lgd2+mzsyfKDnU6JaQ+qyHzgh1FA99NTdsTlHWhLA5Ja lviv4NkmWuDAqdNxvDuwzdtJVpnOU78KC7JTkGeDYR5Kf+1pXasPqiNC6D11R6i/X4Hs wAlOcMDkqHZ3W3Q2zJpzU5EXuSaiGQsk6FuyU=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4C6563E6.6090109@xxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1BEA8649F0C00540AB2811D7922ECB6C9338B4CD@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <AANLkTin0RX1WM_5NQP2BCGMR4LpbEC-ejPvYS6a5TLxm@xxxxxxxxxxxxxx> <AANLkTinLBLN-+YtJ9hSUbKzbL6O9XVENtKLKvH7YF=OO@xxxxxxxxxxxxxx> <4C6563E6.6090109@xxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Hi Andre,

Thanks for the reviews. I am going to be very busy this week, so I
will take this up immediately after that.
Also, as you mentioned elsewhere, we can have this code checked-in in
an acceptable state and you could
push your rebased changes on that.

thanks
dulloor

On Fri, Aug 13, 2010 at 8:25 AM, Andre Przywara <andre.przywara@xxxxxxx> wrote:
> Dulloor wrote:
>>
>> Implement basic utility functions to manipulate bitmasks. Used in later
>> patches.
>>
>> -dulloor
>>
>> Signed-off-by : Dulloor <dulloor@xxxxxxxxx>
>>
> In general this looks OK, although a bit too sophisticated for my
> personal taste. Only some minor comments:
>
> It seems that these functions are somewhat generic, so it may be worth
> to create a generic interface instead and somehow tie the connection
> to xenctl_cpumap later with the instantiation. The generic functions
> could be prefixed with xc_bitmap_*.
> QEMU is about to also get a generic bitmap library:
> http://lists.gnu.org/archive/html/qemu-devel/2010-08/msg00517.html
> maybe one could leverage this.
>
>> +++ b/tools/libxc/xc_cpumap.c
>> +void __xc_cpumap_or(struct xenctl_cpumap *dstp,
>> +        struct xenctl_cpumap *src1p, struct xenctl_cpumap *src2p)
>> +{
>> +    uint8_t *dp = xc_cpumap_bits(dstp);
>> +    uint8_t *s1p = xc_cpumap_bits(src1p);
>> +    uint8_t *s2p = xc_cpumap_bits(src2p);
>> +    int nr = XC_BITS_TO_BYTES(xc_cpumap_len(dstp));
>> +    int k;
>> +    for (k=0; k<nr; k++)
>> +        dp[k] = s1p[k] | s2p[k];
>> +}
>
> Shouldn't we observe the special case with different source length here? If
> one bitmap contains garbage after it's end, then the result would be bogus.
> I think the bitmap or'ing should be stopped after both input bitmaps came to
> an end. I think xc_cpumap_setall() does it correct.
>
>> +
>> +static inline uint8_t hweight8(uint8_t w)
>> +{
>> +    uint8_t res = (w & 0x55) + ((w >> 1) & 0x55);
>> +    res = (res & 0x33) + ((res >> 2) & 0x33);
>> +    return (res & 0x0F) + ((res >> 4) & 0x0F);
>> +}
>> +
>> +int __xc_cpumap_weight(struct xenctl_cpumap *srcp)
>> +{
>> +    const uint8_t *sp = xc_cpumap_bits(srcp);
>> +    int k, w = 0, lim = XC_BITS_TO_BYTES(xc_cpumap_len(srcp));
>> +    for (k=0; k <lim; k++)
>> +        w += hweight8(sp[k]);
>> +    return w;
>> +}
>
> We should stop counting after hitting the maximum specified length,
> otherwise possible garbage bits would be counted in.
>
>> +
>> +/* xenctl_cpumap print function */
>> +#define CHUNKSZ        8
>> +#define roundup_power2(val,modulus)    (((val) + (modulus) - 1) &
>> ~((modulus) - 1))
>> +
>> +int __xc_cpumap_snprintf(char *buf, unsigned int buflen,
>> +                                        const struct xenctl_cpumap
>> *cpumap)
>> +{
>> +    const uint8_t *maskp = xc_cpumap_bits(cpumap);
>> +    int nmaskbits = xc_cpumap_len(cpumap);
>> +       int i, word, bit, len = 0;
>> +       unsigned long val;
>> +       const char *sep = "";
>> +       int chunksz;
>> +       uint8_t chunkmask;
>> +
>> +       chunksz = nmaskbits & (CHUNKSZ - 1);
>> +       if (chunksz == 0)
>> +               chunksz = CHUNKSZ;
>> +
>> +       i = roundup_power2(nmaskbits, CHUNKSZ) - CHUNKSZ;
>> +       for (; i >= 0; i -= CHUNKSZ) {
>> +               chunkmask = ((1ULL << chunksz) - 1);
>> +               word = i / XC_BITS_PER_BYTE;
>> +               bit = i % XC_BITS_PER_BYTE;
>> +               val = (maskp[word] >> bit) & chunkmask;
>> +               len += snprintf(buf+len, buflen-len, "%s%0*lx", sep,
>> +                       (chunksz+3)/4, val);
>> +               chunksz = CHUNKSZ;
>
> Isn't that line redundant?
>>
>> +               sep = ",";
>> +       }
>> +       return len;
>> +}
>
> Regards,
> Andre.
>
> --
> Andre Przywara
> AMD-Operating System Research Center (OSRC), Dresden, Germany
> Tel: +49 351 448-3567-12
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>