Question regarding VM

[srivatsa bhat]

Hello all,

In servers/vm/arch/i386/pagetable.c, in the function pt_init() we find the following code:

/* Initial (current) range of our virtual address space. */
    lo = CLICK2ABS(vmprocess->vm_arch.vm_seg[T].mem_phys);
    hi = CLICK2ABS(vmprocess->vm_arch.vm_seg[S].mem_phys +
            vmprocess->vm_arch.vm_seg[S].mem_len);                 
...

if(lo < VM_PROCSTART) {
        moveup = VM_PROCSTART - lo;
...
}

...

/* Move segments up too. */
    vmprocess->vm_arch.vm_seg[T].mem_phys += ABS2CLICK(moveup);
    vmprocess->vm_arch.vm_seg[D].mem_phys += ABS2CLICK(moveup);
    vmprocess->vm_arch.vm_seg[S].mem_phys += ABS2CLICK(moveup);

...

/* Increase our hardware data segment to create virtual address
 * space above our stack. We want to increase it to VM_DATATOP,
 * like regular processes have.
 */
    extra_clicks = ABS2CLICK(VM_DATATOP - hi);
    vmprocess->vm_arch.vm_seg[S].mem_len += extra_clicks;

 /* We pretend to the kernel we have a huge stack segment to
  * increase our data segment.
  */
...

Here is my question:
I feel the line "extra_clicks..." should have been:
extra_clicks = ABS2CLICK(VM_DATATOP - (hi + moveup));

That is, should we not add 'moveup' to 'hi' and then compute the extra clicks, since 'hi' refers to the top end of VM's address space before VM was moved up?
So shouldn't we be using the new end of VM's address space (the one we get after moving VM by 'moveup') to compute the extra clicks?

I added appropriate print statements in the code and upon testing I found that due to the above "bug" (correct me if I am wrong), the value of CLICK2ABS(vmprocess->vm_arch.vm_seg[S].mem_phys + vmprocess->vm_arch.vm_seg[S].mem_len) [before [S].mem_len is restored to its original length], had gone beyond VM_DATATOP (0xFFFFF000) and had in fact overshot the value that can be accommodated in a 32 bit number!

I have attached screenshots of the outputs with and without adding 'moveup' as explained above. (I have used MINIX 3.1.7 for testing. But the relevant parts of the code in pt_init() are still the same even in the latest code in the trunk.)

Regards,
Srivatsa S. Bhat

[srivatsa bhat]

Well?? Is that not a bug? I would like to kindly request some comments/suggestions/corrections if any.
Thank you.

-Srivatsa S. Bhat

[ben gras]

It does look funny, yes. Assuming your reasoning is right, I think the effect will be to set the vm data hardware segment larger than vm thinks it is. But unless it's causing problems I don't want to drop everything right now to investigate it; I'll investigate and fix it when I have the time, and followup here. Thanks for reporting and showing such detailed interest in the workings of vm :).

[srivatsa bhat]

On Fri, Apr 15, 2011 at 6:11 PM, ben gras <be...@few.vu.nl> wrote:

It does look funny, yes. Assuming your reasoning is right, I think the effect will be to set the vm data hardware segment larger than vm thinks it is. But unless it's causing problems I don't want to drop everything right now to investigate it; I'll investigate and fix it when I have the time, and followup here. Thanks for reporting and showing such detailed interest in the workings of vm :).

--

Thank you so much for your kind reply. I had posted 2 more such "bugs" on the Google Group. I will activate those threads again for your kind attention.

Thank you very much.

-Srivatsa S. Bhat

[Ben Gras]

BTW this code is entirely gone in the new segmentless world.