Re: [MirageOS-devel] wireshark capture of failed download from mirage-www on ARM

[Thomas Leonard <talex5@xxxxxxxxx>]

On 23 July 2014 13:10, Anil Madhavapeddy <anil@xxxxxxxxxx> wrote:
> On 23 Jul 2014, at 02:46, Dave Scott <Dave.Scott@xxxxxxxxxx> wrote:
>
>> On 23 Jul 2014, at 08:10, Thomas Leonard <talex5@xxxxxxxxx> wrote:
>>
>>> On 22 July 2014 13:17, Anil Madhavapeddy <anil@xxxxxxxxxx> wrote:
>>>> On 22 Jul 2014, at 02:01, Thomas Leonard <talex5@xxxxxxxxx> wrote:
>>>>
>>>>> On 22 July 2014 09:14, Anil Madhavapeddy <anil@xxxxxxxxxx> wrote:
>>>>>> On 22 Jul 2014, at 01:10, Thomas Leonard <talex5@xxxxxxxxx> wrote:
>>>>>>>
>>>>>>> I noticed I'm getting a lot of these in the dom0 log (dmsg):
>>>>>>>
>>>>>>> Jul 22 08:09:05 cubietruck kernel: [11892.128241]
>>>>>>> xen_add_phys_to_mach_entry: cannot add pfn=0x00079abe ->
>>>>>>> mfn=0x000bed87: pfn=0x000799a6 -> mfn=0x000bed87 already exists
>>>>>>> Jul 22 08:09:05 cubietruck kernel: [11892.128727]
>>>>>>> xen_add_mach_to_phys_entry: cannot add pfn=0x00079bcb ->
>>>>>>> mfn=0x000bed87: pfn=0x000799a6 -> mfn=0x000bed87 already exists
>>>>>>> Jul 22 08:09:05 cubietruck kernel: [11892.133264]
>>>>>>> xen_add_phys_to_mach_entry: cannot add pfn=0x00079b70 ->
>>>>>>> mfn=0x000bed87: pfn=0x00079bd7 -> mfn=0x000bed87 already exists
>>>>>>> Jul 22 08:09:09 cubietruck kernel: [11896.041106]
>>>>>>> xen_add_phys_to_mach_entry: cannot add pfn=0x0007999a ->
>>>>>>> mfn=0x000bed87: pfn=0x00079b2b -> mfn=0x000bed87 already exists
>>>>>>
>>>>>> That looks like this bug in Xen/ARM/netback:
>>>>>> http://lists.xenproject.org/archives/html/xen-users/2014-07/msg00067.html
>>>>>
>>>>> Aha! This seems to fix it (I can browse mirage-www without huge delays):
>>>>>
>>>>> diff --git a/lib/netif.ml b/lib/netif.ml
>>>>> index 4e02e5e..3cc7760 100644
>>>>> --- a/lib/netif.ml
>>>>> +++ b/lib/netif.ml
>>>>> @@ -455,7 +455,7 @@ let writev nf pages =
>>>>>        lwt rest_th = xmit other_pages in
>>>>>        (* All fragments are now written, we can now notify the backend *)
>>>>>        Lwt_ring.Front.push nf.t.tx_client (notify nf.t);
>>>>> -         return ()
>>>>> +         join rest_th
>>>>>   )
>>>>>
>>>>> let wait_for_plug nf =
>>>>>
>>>>> Not very efficient, but at least it works! Looks like the Xen people
>>>>> will fix it at some point.
>>>>
>>>> That's going to serialize requests on the TX ring, isn't it?  Might
>>>> be worth making conditional via a feature flag if it's going to take
>>>> x86 transmit performance down.

I've done some tests, and this absolutely kills performance. Sending a
load of simple 1K UDP packets, the CubieTruck sends at 28 KB/s with
this patch and 1,531 KB/s with the wait outside the mutex.

However, this still isn't quite right, because in the fast case I send
all the packets and then wait for them all to complete. But really I'd
like the sender to slow down when the receiver is getting behind,
rather than allocating ever more memory to the queue. Doing that will
require an API change I think.

>>> FLOW.writev is defined as:
>>>
>>> val writev : flow -> buffer list -> [`Ok of unit | `Eof | `Error of error ] 
>>> io
>>> (** [writev flow buffers] will block until the contents of [buffer list]
>>>     are all successfully transmitted to the remote endpoint. The result
>>>     [`Ok ()] indicates success, [`Eof] indicates that the connection is now
>>>     closed and [`Error] indicates some other error. *)
>>>
>>> I took this to mean that when you get `Ok back, the buffers are no
>>> longer needed and can be used for something else. Looking at the
>>> implementation, it seems that `Ok just means that the receiver has
>>> been given access to the buffers, but might not have read them yet.
>>>
>>> So actually, I think this change might be correct for x86 too,
>>> although perhaps it should be moved out of the mutex.
>>
>> Yeah, I think youâre right!
>>
>
> Agreed.  Could you drop in a pull request for that on mirage/mirage-net-xen, 
> Thomas?  I'll test and merge on my Cubie to use for the OSCON talk tomorrow 
> (now approaching fast :)
>
>> I think itâs time to think about running regular performance benchmarks. Now 
>> that we have the little testvm[1] which can be controlled over 
>> vchan/HTTP[2], we could add the iperf example code to it and then set up 
>> some test cases. If we made the tests easy to script then we might be able 
>> to run them on the same hardware we use for Xenserver and benchmark them 
>> against Windows and Linux.
>>
>
> I'd missed this!  I can certainly arrange to have a dedicated Ubuntu box run 
> the tests regularly here in the Computer Lab and publish them to Git, if you 
> can provide an appropriate shell harness.  Testing on both XenServer and 
> Ubuntu's Xen should give us some spread in the software versions.
>
>> I wonder if weâll have to expand the FLOW interface a bit. I notice thereâs 
>> a trend towards offering applications more control over queues (e.g. VMDQ) 
>> so we might want to do the same. This would allow an app to âpushâ requests 
>> onto the queue and block (which would itself block if the queue max size was 
>> hit) and then wait for the work to be completed and the buffers free.
>
> Yeah, and Core/Async Pipe model is a good way to model the interface here.

This?

https://ocaml.janestreet.com/ocaml-core/111.21.00/doc/async/#Std.Pipe

Sounds reasonable. I guess we should make the cstructs (forever)
read-only once they've been submitted, and rely on the GC to recycle
them. Then the caller doesn't have to care when they're freed but we
can still provide back-pressure. This is basically what we had before.
The changes would be:

1) change the documentation to say that writev only blocks until it's
ready for more packets to be submitted, not until these ones have been
sent.
2) document that buffers cannot be reused.
3) add a way to make a cstruct forever immutable.

> -anil
>
>
>>
>> Cheers,
>> Dave
>>
>> [1] https://github.com/mirage/xen-testvm
>> [2] https://github.com/mirage/testvm-idl
>>
>>>
>>> (if you call writev with a singleton list then it does wait inside the
>>> mutex already)




-- 
Dr Thomas Leonard        http://0install.net/
GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/mirageos-devel