Date: Fri, 10 Mar 2006 14:35:45 -0800
From: Andrew Morton <akpm@osdl.org>
To: Con Kolivas <kernel@kolivas.org>
Cc: linux-kernel@vger.kernel.org, ck@vds.kolivas.org
Subject: Re: [PATCH] mm: Implement swap prefetching tweaks
Message-Id: <20060310143545.74a9a92a.akpm@osdl.org>
In-Reply-To: <200603102054.20077.kernel@kolivas.org>
References: <200603102054.20077.kernel@kolivas.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1452
Lines: 49

Con Kolivas <kernel@kolivas.org> wrote:
>
> +	/*
> +	 * get_page_state is super expensive so we only perform it every
> +	 * SWAP_CLUSTER_MAX prefetched_pages.

nr_running() is similarly expensive btw.

> 	 * We also test if we're the only
> +	 * task running anywhere. We want to have as little impact on all
> +	 * resources (cpu, disk, bus etc). As this iterates over every cpu
> +	 * we measure this infrequently.
> +	 */
> +	if (!(sp_stat.prefetched_pages % SWAP_CLUSTER_MAX)) {
> +		unsigned long cpuload = nr_running();
> +
> +		if (cpuload > 1)
> +			goto out;

Sorry, this is just wrong.  If swap prefetch is useful then it's also
useful if some task happens to be sitting over in the corner calculating
pi.

What's the actual problem here?  Someone's 3d game went blippy?  Why?  How
much?  Are we missing a cond_resched()?

> +		cpuload += nr_uninterruptible();
> +		if (cpuload > 1)
> +			goto out;

Not sure about this either.  


> +		if (ns->last_free) {
> +			if (ns->current_free + SWAP_CLUSTER_MAX <
> +				ns->last_free) {
> +					ns->last_free = ns->current_free;
>  					node_clear(node,
>  						sp_stat.prefetch_nodes);
>  					continue;
>  			}
>  		} else

That has an extra tabstop.
-
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/