On Tue, 2006-06-27 at 14:54 +0200, Franck Bui-Huu wrote:
> static void __init free_bootmem_core(bootmem_data_t *bdata, unsigned
> long addr,
> unsigned long size)
> {
> + unsigned long sidx, eidx;
> unsigned long i;
> - unsigned long start;
> +
> /*
> * round down end of usable mem, partially free pages are
> * considered reserved.
> */
> - unsigned long sidx;
> - unsigned long eidx = (addr + size -
> bdata->node_boot_start)/PAGE_SIZE;
> - unsigned long end = (addr + size)/PAGE_SIZE;
> -
> BUG_ON(!size);
> - BUG_ON(end > bdata->node_low_pfn);
> + BUG_ON(PFN_DOWN(addr + size) > bdata->node_low_pfn);
In general, I like these kinds of conversions. But, in this case, I
think it makes the code harder to read. Those intermediate variables
are really nice and I think they make the code much more readable.
Do you really prefer:
BUG_ON(PFN_DOWN(addr + size) > bdata->node_low_pfn)
over
BUG_ON(end > bdata->node_low_pfn);
Also, these do a bit more than just conversions to using the pfn/page
macros. With this much churn, it is more than possible that bugs can
creep in. How about a bit more restrictive conversion to the PFN_
macros, first?
Oh, and if you're going to chew through it later, feel free to make
things like sidx into decent variable names. ;)
Is everybody else OK with this code churn? It doesn't appear that there
is too much in -mm pending in this area.
-- Dave
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
