Re: [Xen-devel] Re: [PATCH 5/7] mm: New XENMEM space, XENMAPSPACE_gmfn_range

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.11.11 at 10:30, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote:
> On 11/11 09:21, Jan Beulich wrote:
>> >>> On 11.11.11 at 10:13, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
>> > On Fri, 2011-11-11 at 08:09 +0000, Jan Beulich wrote:
>> >> >>> On 10.11.11 at 18:37, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote:
>> >> > On 10/11 12:47, Jan Beulich wrote:
>> >> >> >>> On 10.11.11 at 12:35, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> 
>> >> >> >>> wrote:
>> >> >> >@@ -4716,6 +4748,17 @@ long arch_memory_op(int op, 
>> >> >> >XEN_GUEST_HANDLE(void) 
>> >> > arg)
>> >> >> >         }
>> >> >> > 
>> >> >> >         rc = xenmem_add_to_physmap(d, &xatp);
>> >> >> >+        if ( rc == -EAGAIN )
>> >> >> 
>> >> >>         if ( rc )
>> >> >> 
>> >> >> >+        {
>> >> >> >+            if ( copy_to_guest(arg, &xatp, 1) )
>> >> >> >+            {
>> >> >> >+                rcu_unlock_domain(d);
>> >> >> >+                return -EFAULT;
>> >> >> >+            }
>> >> >> 
>> >> >>         }
>> >> >>         if ( rc == -EAGAIN )
>> >> >> 
>> >> >> (with some room for further simplification). Without that (or the 
>> >> >> minimal
>> >> >> alternative of copying back just .size or yet some other mechanism), as
>> >> >> pointed out before, the caller won't have a way to know how far into
>> >> >> the batch things succeeded.
>> >> >> 
>> >> > 
>> >> > In xenmem_add_to_physmap I modify xatp in place so when I exit this
>> >> > function xatp will contain the updated value (new start value in
>> >> > .gpfn and .idx, how far do I need to go in .size).
>> >> > 
>> >> > The idea here was to call the copy_to_guest only when we got preempted.
>> >> > If I copy xatp back to the guest I should get the updated value
>> >> > in xatp from the copy_from_guest when I'll be called by the 
>> >> > continuation.
>> >> 
>> >> I understand the continuation aspect. But you appear to have not read
>> >> my comments completely: I'm asking how your caller, in the event of
>> >> failure, would know how much of the batch was processed successfully.
>> > 
>> > For this sort of flush operation can the caller assume that failure
>> > means nothing was flushed, since a flush can always be repeated?
>> 
>> This is not just a flush - instead, the flush is just a necessary sub-
>> operation of what is being done here. I don't think the actual add-
>> to-physmap should be repeated. And even if it can, at least for
>> diagnostic/debugging purposes knowing where things failed is rather
>> useful.
>> 
> 
> Ok, I'll do the copy_to_guest if rc isn't 0. Do you think I should do it
> for all the spaces or only for gmfn_range?

I'd prefer doing it just for the latter - the others have no outputs,
and their input could legitimately (albeit unlikely) live in read-only
memory.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel