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 ormake it a real static inline function.

Should this be put in mm/page_alloc.c? It is, after all, a special-purposepage allocator. That way you wouldn't have to export zone_statistics andbuffered_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 valuechecks in kprefetchd() slightly confusing.