WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

>>> 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