>>> On 30.09.16 at 17:02, <roger....@citrix.com> wrote:
> On Fri, Sep 30, 2016 at 07:21:41AM -0600, Jan Beulich wrote:
>> >>> On 30.09.16 at 13:27, <roger....@citrix.com> wrote:
>> > On Thu, Sep 29, 2016 at 08:18:36AM -0600, Jan Beulich wrote:
>> >> >>> On 27.09.16 at 17:57, <roger....@citrix.com> wrote:
>> >> > +{
>> >> > + int rc;
>> >> > +
>> >> > + while ( nr_pages > 0 )
>> >> > + {
>> >> > + rc = (map ? map_mmio_regions : unmap_mmio_regions)
>> >> > + (d, _gfn(pfn), nr_pages, _mfn(pfn));
>> >> > + if ( rc == 0 )
>> >> > + break;
>> >> > + if ( rc < 0 )
>> >> > + {
>> >> > + printk(XENLOG_ERR
>> >> > + "Failed to %smap %#lx - %#lx into domain %d memory
>> >> > map:
> %d\n",
>> >> > + map ? "" : "un", pfn, pfn + nr_pages, d->domain_id,
>> >> > rc);
>> >> > + return rc;
>> >> > + }
>> >> > + nr_pages -= rc;
>> >> > + pfn += rc;
>> >> > + process_pending_softirqs();
>> >> > + }
>> >> > +
>> >> > + return rc;
>> >>
>> >> The way this is coded it appears to possibly return non-zero even in
>> >> success case. I think this would therefore better be a for ( ; ; ) loop.
>> >
>> > I don't think this is possible, {un}map_mmio_regions will return < 0 on
>> > error, > 0 if there are pending pages to map, and 0 when all the requested
>> > pages have been mapped successfully.
>>
>> Right - hence the "appears to" in my reply; it took me a while to
>> figure it's not actually possible, and hence my desire to make this
>> more obvious to the reader.
>
> Ah, OK, I misunderstood you then. What about changing the last return rc
> to return 0? This would make it more obvious, because I'm not really sure a
> for loop would change much (IMHO the problem is the return semantics used by
> {un}map_mmio_regions).
Well, my suggestion was not to use "a for loop" but specifically
"for ( ; ; )" to clarify the only loop exit condition is the single break
statement. That break statement could of course also become a
"return 0". I'd rather not see the return at the end of the function
become "return 0"; if anything (i.e. if you follow my suggestion) it
could be deleted altogether.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
