2008/5/20 Jeff Dike <jdike@addtoit.com>:
quoted text
> On Tue, May 20, 2008 at 08:42:54PM +0100, Tom Spink wrote:
>> And this is what I did:
>
> Looks OK, but I need a Signed-off-by.
>
> Some style comments, which I can take care of if you don't feel like it:
>        there's only one use of PROC_MMAP_MIN_ADDR, so you might as
> well inline the filename
>        the stat is unnecessary - open failing will tell you what you
> need to know
>        the strtoul needs some error checking
>        I'd change the mmap_min_addr[8] to [sizeof("12345678")] to
> make it clear what's expected to fit into it
>
> On a less-stylistic note, this thing does have to work in the absence
> of /proc since UML is commonly run inside chroot.  So, I'm thinking we
> still need the loop looking for the bottom of the address space in
> this case.  And, if you have it, you might as well use it, as that's
> less code, and it makes sure the loop is exercised.  That grieves me a
> little, as I'd like to just read mmap_min_addr and be done with it in
> the normal case.
>
>                        Jeff
>
> --
> Work email - jdike at linux dot intel dot com
>

I was re-writing the patch, but after reading over the function, is
the return value correct? I did a quick run through on paper about
what would happen if the (real) bottom = 5 and the (real) top = 15,
and the (initial) top = 20.  So this is a size of 10 pages, which is
what the function should return... but:

top = 20, bottom = 5
test = 12
bottom = 12
(top - bottom) = 8

top = 20, bottom = 12
test = 16
top = 16
(top - bottom) = 4

top = 16, bottom = 12
test = 14
bottom = 14
(top - bottom) = 2

top = 16, bottom = 14
test = 15
bottom = 15
(top - bottom) = 1

<loop exits>

return 16 << UM_KERN_PAGE_SHIFT;

Hmm.  Are my calculations wrong?

-- 
Tom Spink
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/