Re: [PATCH] vm - swap_prefetch-15

[Pekka Enberg <penberg@cs.helsinki.fi>]

Hi,

On 10/7/05, Con Kolivas <kernel@kolivas.org> wrote:
> Good point, thanks! Any and all feedback is appreciated.

Well, since you asked :-)

> +/*
> + * How many pages to prefetch at a time. We prefetch SWAP_CLUSTER_MAX *
> + * swap_prefetch per PREFETCH_INTERVAL, but prefetch ten times as much at a
> + * time in laptop_mode to minimise the time we keep the disk spinning.
> + */
> +#define PREFETCH_PAGES()     (SWAP_CLUSTER_MAX * swap_prefetch * \
> +                                     (1 + 9 * laptop_mode))

This looks strange. Please either drop the parenthesis from PREFETCH_PAGES or
make it a real static inline function.

> +/*
> + * Find the zone with the most free pages, recheck the watermarks and
> + * then directly allocate the ram. We don't want prefetch to use
> + * __alloc_pages and go calling on reclaim.
> + */
> +static struct page *prefetch_get_page(void)
> +{

Should this be put in mm/page_alloc.c? It is, after all, a special-purpose
page allocator. That way you wouldn't have to export zone_statistics and
buffered_rmqueue.

> +/*
> + * trickle_swap is the main function that initiates the swap prefetching. It
> + * first checks to see if the busy flag is set, and does not prefetch if it
> + * is, as the flag implied we are low on memory or swapping in currently.
> + * Otherwise it runs till PREFETCH_PAGES() are prefetched.
> + * This function returns 1 if it succeeds in a cycle of prefetching, 0 if it
> + * is interrupted or -1 if there is nothing left to prefetch.
> + */
> +static int trickle_swap(void)
> +{

This could perhaps use a three-state enum as return value. I find return value
checks in kprefetchd() slightly confusing.

                                Pekka
-
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/