Re: [PATCH] 2/2 Prezeroing large blocks of pages during allocation

[Dave Hansen]

On Sun, 2005-02-27 at 13:43 +0000, Mel Gorman wrote:
> +		/*
> +		 * If this is a request for a zero page and the page was
> +		 * not taken from the USERZERO pool, zero it all
> +		 */
> +		if ((flags & __GFP_ZERO) && alloctype != ALLOC_USERZERO) {
> +			int zero_order=order;
> +
> +			/*
> +			 * This is important. We are about to zero a block
> +			 * which may be larger than we need so we have to
> +			 * determine do we zero just what we need or do
> +			 * we zero the whole block and put the pages in
> +			 * the zero page. 
> +			 *
> +			 * We zero the whole block in the event we are taking
> +			 * from the KERNNORCLM pools and otherwise zero just
> +			 * what we need. The reason we do not always zero
> +			 * everything is because we do not want unreclaimable
> +			 * pages to leak into the USERRCLM and KERNRCLM 
> +			 * pools
> +			 *
> +			 */
> +			if (alloctype != ALLOC_USERRCLM &&
> +			    alloctype != ALLOC_KERNRCLM) {
> +				area = zone->free_area_lists[ALLOC_USERZERO] +
> +					current_order;
> +				zero_order = current_order;
> +			}
> +
> +			
> +			spin_unlock_irqrestore(&zone->lock, *irq_flags);
> +			prep_zero_page(page, zero_order, flags);
> +			inc_zeroblock_count(zone, zero_order, flags);
> +			spin_lock_irqsave(&zone->lock, *irq_flags);
> +
> +		}
> +
>  		return expand(zone, page, order, current_order, area);
>  	}
>  

I think it would make sense to put that in its own helper function.
When comments get that big, they often reduce readability.  The only
outside variable that gets modified is "area", I think.

So, a static inline:

	area = my_new_function_with_the_huge_comment(zone, ..., area);

Should give the same behavior, generated code, and be a bit easier on
the eyes.  

BTW, what kernel does this apply against?  Is linux-2.6.11-rc4-v18 the
same as bk18?

-- Dave

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