lkml.org 
[lkml]   [2006]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 6/7] bootmem: use pfn/page conversion macros
From
Date
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 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/

\
 
 \ /
  Last update: 2006-06-27 21:29    [W:6.077 / U:0.432 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site