Re: make swappiness safer to use

Andrew Morton
2007-07-31T22:12:44Z

On Tue, 31 Jul 2007 23:52:28 +0200
Andrea Arcangeli <andrea@suse.de> wrote:

> + swap_tendency += zone_page_state(zone, NR_ACTIVE) /
> + (zone_page_state(zone, NR_INACTIVE) + 1)
> + * (vm_swappiness + 1) / 100
> + * mapped_ratio / 100;

I must say, that's a pretty ugly-looking statement. For a start, the clause

* (vm_swappiness + 1) / 100

always evaluates to zero. The L->R associativity prevents that, but the
layout is super-misleading, no?

And it matters - the potential for overflow and rounding errors here is
considerable. Let's go through it. Probably 32-bit is the problem.

zone_page_state(zone, NR_ACTIVE) /

0 -> 8,000,000

(zone_page_state(zone, NR_INACTIVE) + 1)

min: 1, max: 8,000,000

* (vm_swappiness + 1)

min: 1, max: 101

total min: 1, total max: 800,000,000

/ 100

total min: 0, total max: 8,000,000

* mapped_ratio

total min: 0, total max: 800,000,000

/ 100;

total min: 0, total max: 8,000,000

then we divide zone_page_state(zone, NR_ACTIVE) by this value.

We can get a divide-by-zero if zone_page_state(zone, NR_INACTIVE) is
sufficiently small, I think? At least, it isn't obvious that we cannot.

I suspect that we can get a value >100, too. Especially when we add it to
the existing value of swap_tendency, but I didn't think about it too hard.

Want to see if we can present that expression in a more logical fashion, and
be more careful about the underflows and overflows, and fix the potential
divide-by-zero?

Thanks.