Re: [Xen-devel] [PATCH v6 09/16] x86/hvm: limit reps to avoid the need to handle retry

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.07.15 at 14:50, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 09 July 2015 13:04
>> >>> On 09.07.15 at 13:11, <Paul.Durrant@xxxxxxxxxx> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >> Sent: 09 July 2015 11:05
>> >> >>> On 03.07.15 at 18:25, <paul.durrant@xxxxxxxxxx> wrote:
>> >> > @@ -287,17 +271,56 @@ static int hvmemul_do_io_addr(
>> >> >      bool_t is_mmio, paddr_t addr, unsigned long *reps,
>> >> >      unsigned int size, uint8_t dir, bool_t df, paddr_t ram_gpa)
>> >> >  {
>> >> > -    struct page_info *ram_page;
>> >> > +    struct vcpu *v = current;
>> >> > +    unsigned long ram_gmfn = paddr_to_pfn(ram_gpa);
>> >> > +    unsigned int page_off = ram_gpa & (PAGE_SIZE - 1);
>> >> > +    struct page_info *ram_page[2];
>> >> > +    int nr_pages = 0;
>> >>
>> >> unsigned int
>> >
>> > No, it's intentionally signed because the unwind code at the end of the
>> > function is:
>> >
>> >     while ( --nr_pages >= 0 )
>> >         hvmemul_release_page(ram_page[nr_pages]);
>> >
>> > I.e. the loop terminates when nr_pages gets to -1.
>> 
>>      while ( nr_pages-- )
>>          hvmemul_release_page(ram_page[nr_pages]);
>> 
> 
> But that wouldn't be correct, since the loop would try to release 
> ram_page[1] and ram_page[0] in the case where only ram_page[0] had been 
> acquired. It would have to be:
> 
>       while ( nr_pages )
>           hvmemul_release_page(ram_page[--nr_pages]);

Unless you need the value of nr_pages after the loop, I can't see
the difference between your and my proposal - the decrement in
both cases happens between determining whether to continue the
loop and accessing the intended array element.

> which I'll do, since you so dislike signed ints.

Sounds like you're not understanding why. Just look at the
generated code when using signed vs unsigned int as array
index.

Jan


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