Re: try_to_swap_out() return value problem?

[Linus Torvalds]

On Thu, 4 Jan 2001, Marcelo Tosatti wrote:
> 
> Your latest changes to try_to_swap_out() does not seem to be obviously
> correct.

Heh. What a polite way of saying that you think I'm full of sh*t ;)

> Having swap_cnt == 0 does not necessarily mean that we successfully freed
> a page (look at the page aging and locking checks in try_to_swap_out).

No.

What it means, though, is that we did finish what we set out to do.
Nothing more.

> Now refill_inactive() relies on the assumption that swap_out() returning 
> 1 means we successfully freed a page:
> 
>                 while (swap_out(priority, gfp_mask)) {
>                         made_progress = 1;
>                         if (--count <= 0)
>                                 goto done;
>                 }

Well, refill-inactive really relies on the assumption that _some_ progress
was made, which was certainly true. "swap_out()" returning 1 has not
actually meant that a page has been free'd for a _long_ long time: it's
meant that _something_ changed (ie "we added one page to the swap cache"
or similar).

I agree that the return value of swap_out() is fairly meaningless. It's
been fairly meaningless for a long time now, and it's entirely possible
that the "while (swap_out())" loop should be just something like

	/* Scan the VM space, try to clean up the page tables a bit */
	for (i = 0 ; i <= nr_threads >> priority; i++)
		swap_out(gfp_mask);

which just tries to move pages out of the page tables, so that the _real_
page freeing logic (ie page_launder() and friends) have more to work with.
This is especially true these days as swap_out() really doesn't do any IO
at all (and has thus finally become completely misnamed: it's not so much
swapping stuff out, as just aging the page tables).

As it is, I think that the above is fairly close to what the code
effectively ends up actually _doing_. Even if it's not really written that
way.

		Linus