Re: Fixed Mirage

[Anil Madhavapeddy <anil@xxxxxxxxxx>]

Awesome! I'm pulling to trunk now and trying this out, as I could reproduce the 
hanging bug before on a static IP.

I noticed this:
https://github.com/mirage/mirage-platform/commit/d22f60048890e0b253788a554d6ce00021f76f4e

Doesn't this totally nullify the point of Console.log_s blocking?  With the 
switch to Lwt.async, multiple log_s calls can now interleave and corrupt the 
console ring (whereas previously they would be synchronous).  This would 
require a mutex around the console ring to be totally safe (and Console.log 
exists for when you want to be non-blocking).

And just a minor thing in the same commit, but the change to "if num > 0" in 
refill_requests also causes one less event notification, and so isn't quite a 
noop.  That's worth making a separate commit from the interface adaptation also.

A good general workflow for making changes is:

- make many commits and one pull request, and then merge it if you don't want 
code review (e.g. an urgent fix).  This gives everyone watching the repo an 
explicit mail about the series.

- even if you make a lot of changes in one go, split up the commits by using 
"git add -p" to break up a patch into smaller patches interactively.

- you will love the smaller commits later on when you are tracking down a bug, 
since you can use "git bisect" and some shell scripts to automatically narrow 
it down to one commit in many cases.  This requires slightly more automated 
unit tests for us.

-anil

On 11 Jul 2013, at 21:33, Vincent Bernardoff <vb@xxxxxxxxxxxxxx> wrote:

> Dear all,
> 
> I have pushed patches to resp. mirage-platform and mirage-net that fixes the 
> DHCP bug (and should fix Balraj issues as well).
> 
> The bug was caused by my modification of Io_page: I replaced the function 
> get_n that was allocating n Bigarrays of size one page by a function that was 
> allocating one Bigarray of size n pages. I guess that the entire Bigarray was 
> garbage collected when the first of its page was not referenced anymore, and 
> that was causing the issue.
> 
> Balraj, please could you run your tests with this new version, and tell me if 
> it works for you ?
> 
> At least it fixes DHCP completely.
> 
> I broke the UNIX backend temporarily, I will fix it tomorrow.
> 
> Good evening,
> 
> Vincent
>