[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] Fix a BUG_ON issue




> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, August 29, 2016 7:51 PM
> To: Wu, Feng <feng.wu@xxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxx
> Subject: Re: [PATCH] Fix a BUG_ON issue
> 
> >>> On 29.08.16 at 11:14, <feng.wu@xxxxxxxxx> wrote:
> > --- a/xen/drivers/passthrough/io.c
> > +++ b/xen/drivers/passthrough/io.c
> > @@ -243,7 +243,7 @@ static struct vcpu *vector_hashing_dest(const struct
> domain *d,
> >          for ( i = 0; i <= mod; i++ )
> >          {
> >              idx = find_next_bit(dest_vcpu_bitmap, d->max_vcpus, idx) + 1;
> > -            BUG_ON(idx >= d->max_vcpus);
> > +            BUG_ON(idx > d->max_vcpus);
> >          }
> >
> >          dest = d->vcpu[idx - 1];
> 
> Wouldn't it be better to change the code to
> 
>         unsigned int idx = -1;
> 
>         for ( i = 0; i <= mod; i++ )
>         {
>             idx = find_next_bit(dest_vcpu_bitmap, d->max_vcpus, idx + 1);
>             BUG_ON(idx >= d->max_vcpus);
>         }
> 
>         dest = d->vcpu[idx];

Thanks for the comments, both are good to me, but I slightly prefer this
one. Do I need to send another version?

Thanks,
Feng

> 
> or, not utilizing wrapping
> 
>         unsigned int idx = find_first_bit(dest_vcpu_bitmap, d->max_vcpus);
> 
>         for ( i = 0; i < mod; i++ )
>             idx = find_next_bit(dest_vcpu_bitmap, d->max_vcpus, idx + 1);
> 
>         BUG_ON(idx >= d->max_vcpus);
> 
>         dest = d->vcpu[idx];
> 
> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.