At 12:58 +0200 on 14 May (1431608313), Roger Pau Monné wrote:
> El 14/05/15 a les 11.53, Tim Deegan ha escrit:
> > At 16:25 +0200 on 11 May (1431361528), Roger Pau Monne wrote:
> >> When the caller of paging_log_dirty_op is a hvm guest Xen would choke when
> >> trying to copy the dirty bitmap to the guest because the paging lock is
> >> already held.
> >>
> >> Fix this by independently mapping each page of the guest bitmap as needed
> >> without the paging lock held.
> >>
> >> Signed-off-by: Roger Pau Monné <roger....@citrix.com>
> >
> > This looks mostly correct, but OTOH we now seem to have two nested
> > stop-and-retry mechsnisms in one function. I think this would be
> > cleaner if here:
> >
> >> + if ( pages >> (3 + PAGE_SHIFT) !=
> >> + index_mapped >> (3 + PAGE_SHIFT) )
> >> {
> >> - rv = -EFAULT;
> >> - goto out;
> >> + /* We need to map next page */
> >> + d->arch.paging.preempt.log_dirty.i4 = i4;
> >> + d->arch.paging.preempt.log_dirty.i3 = i3;
> >> + d->arch.paging.preempt.log_dirty.i2 = i2;
> >> + d->arch.paging.preempt.log_dirty.done = pages;
> >> + paging_unlock(d);
> >> + unmap_dirty_bitmap(dirty_bitmap, page);
> >
> > we set the rest of the preempt state, like so:
> >
> > d->arch.paging.preempt.dom = current->domain;
> > d->arch.paging.preempt.op = sc->op;
> > resuming = 1;
> >
> > and bailed to the very top of the function. I think we might also
> > need a hypercall_preempt_check() here as well, so that this restart
> > path doesn't stop us ever reaching the preempt_checks below.
>
> OK, I don't mind adding a preempt check here, although I think we would
> hit the ones below anyway as we go scanning the dirty bitmap even if we
> have to restart.
Oh, so we do. No need for the extra check here, then. But I'd still
prefer restarting the whole function to having a second restart point.
Cheers,
Tim.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
