Re: [PATCH v8 02/11] IOMMU/MMU: enhance the call trees of IOMMU unmapping and mapping

["Xu, Quan" <quan.xu@intel.com>]

On June 14, 2016 12:37 AM,  George Dunlap <George.Dunlap@eu.citrix.com> wrote:
> On Mon, Jun 13, 2016 at 4:17 PM, Xu, Quan <quan.xu@intel.com> wrote:
> > From: Quan Xu <quan.xu@intel.com>
> >
> > When IOMMU mapping is failed, we issue a best effort rollback,
> > stopping IOMMU mapping, unmapping the previous IOMMU maps and
> then
> > reporting the error up to the call trees. When rollback is not
> > feasible (in early initialization phase or trade-off of complexity)
> > for the hardware domain, we do things on a best effort basis, only throwing
> out an error message.
> >
> > IOMMU unmapping should perhaps continue despite an error, in an
> > attempt to do best effort cleanup.
> >
> > Signed-off-by: Quan Xu <quan.xu@intel.com>
> > Reviewed-by: Jan Beulich <jbeulich@suse.com>
> > Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > Acked-by: Kevin Tian <kevin.tian@intel.com>
> 
> Acked-by: George Dunlap <george.dunlap@citrix.com>
> 
> Phew!
> 
> One comment...
> 
> > +                        while ( i-- )
> > +                            /*
> > +                             * IOMMU unmapping should perhaps continue despite an
> > +                             * error in an attempt to do best effort cleanup, and
> > +                             * consume the error as __must_check annotation.
> > +                             */
> > +                            if ( iommu_unmap_page(p2m->domain, gfn + i) )
> > +                                continue;
> 
> I'd take out the "perhaps", (since there's no 'perhaps' about it) but other than
> that I think this comment is fine.
> 
> It sounds like Jan had something more along the following in mind:
> 
> /* If statement to satisfy __must_check */
> 
> Either one works.  The shorter one is sufficient, but the longer one isn't too
> much I don't think.
> 
George,
Thanks for your comment.. I think your shorter one is better.

Jan, 
Could you help me review patch 7?   Thanks. 
Then, I can send out next v9 soon and get started to focus on next patch set.

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