Re: [PATCH v8 05/14] arch/arm: unmap partially-mapped I/O-memory regions

[Ian Campbell <ian.campbell@citrix.com>]

On Sun, 2014-05-25 at 17:04 +0100, Julien Grall wrote:
> Hi Arianna,
> 
> On 25/05/14 11:51, Arianna Avanzini wrote:
> > This commit changes the interface of apply_p2m_changes() to accept
> > optionally the pointer to a counter of successfully performed
> > mappings; such a counter is used only in case of INSERT operation.
> > If an error is encountered during the operation, and therefore the
> > mapping is only partially performed, such a counter is useful to
> > let the caller be able to undo what has just been done.
> 
> I don't think you need to introduce another argument to 
> apply_p2m_changes. You can directly call apply_p2m_changes with the 
> whole range.

Due to the sanity checks which Arianna is adding to the unmap/REMOVE
case I think it would no longer be valid to call unmap on the entire
range unless the entire range was successfully mapped.

I was nearly going to suggest that apply_p2m_changes could return the
number of successful mappings so that the caller could check actual ==
expected, but that removes the possibility to provide a specific error
code as well, so I think that is not acceptable.

I can't remember: did we consider and discard the possibility that
apply_p2m_changes should unwind its own progress on failure to INSERT
(perhaps by recursively calling itself with REMOVE)? That would seem to
be what most callers of INSERT would expect I think (despite many of
them passing NULL for this new argument here)

Ian.