Re: Fixed Mirage

["Vincent B." <vb@xxxxxxxxxxxxxx>]

On 12/07/2013 01:05, Anil Madhavapeddy wrote:
> On 12 Jul 2013, at 00:56, "Vincent B."<vb@xxxxxxxxxxxxxx>  wrote:
> > >On 11/07/2013 23:48, Anil Madhavapeddy wrote:
> > > >>Hm, this is still pretty broken and crashes immediately on boot with
> > > >>master/:
> > > >>
> > > >>https://github.com/mirage/mirage-platform/commit/ab88ef636fe3ff06c8638d5ea765ad25bf9610e2
> > >
> > >Whas crashes exactly ? the basic example with DHCP was working well for
> > >me when I commited those changes…
> Are you sure you tested with the changes you committed or with a slightly 
> earlier version?
Yeah, almost certain.

> > > >>Why did you remove the zeroing of the Io_pages?  That breaks
> > > >>everything and the domain crashes as so> on as any shared rings are
> > > >>used, since there is garbage on the ring at start.  I added a
> > > >>memset(block, 0, len) into alloc_pages, and the domain now boots ok,
> > > >>but I still see a lot of packet loss.  Investigating...
> > >
> > >But I added the zeroing in the first place!!! Previously there was no zeroing 
> > at all, I then added it but removed it after I observed that the pages returned by 
> > memalign were actually always zeroed.
> The bug is in a changeset marked "cosmetics" :-)
> https://github.com/mirage/mirage-platform/commit/41f26e337beeb6799f9e04c304480ecca98f0fef
>
> You removed this hunk from allocate_ring:
>
> -  for i = 0 to Cstruct.len x - 1 do
> -    Cstruct.set_uint8 x i 0
> -  done;
>
> which zeros the ring, and Io_pages are also no longer zeroed since you reverted 
> that earlier.  One or the other is needed.  I'd prefer that pages not be zeroed 
> by default, since only the rings really need it.
>
> > >I had the impression that Xen would always zero the pages in order for domains not to 
> > be able to see other domains memory…
> Right, Xen does that with physical memory, but remember that we have a memory slab 
> allocator ("alloc_page" from the xen/runtime/kernel) that is recycling the OS 
> pages for the OCaml runtime to use.  That will only zero pages out if you specifically do 
> so.
Oh I see. I again I did not see that clearly. Yeah, I should definitely 
not have removed this hunk…
Vincent