Re: [PATCH] Autoregulate vm swappiness cleanup

Re: [PATCH] Autoregulate vm swappiness cleanup

[Boszormenyi Zoltan]

Hi,

this patch makes my system even more responsive,
I applied the patch over 2.6.0-test8-mm1.

I have a Glade v1 project that has these source files among others:

$ ls -l callbacks.[ch] interface.[ch]
-rw-rw-r--    1 zozo     zozo       583241 okt 28 13:06 callbacks.c
-rw-rw-r--    1 zozo     zozo        96592 okt 27 22:54 callbacks.h
-rw-rw-r--    1 zozo     zozo      1090912 okt 27 22:54 interface.c
-rw-rw-r--    1 zozo     zozo         1687 okt 27 22:54 interface.h

That said, among others: the projects is *very* large, and is reasonably
modularised before someone says something silly about my programming
capabilities. :-)

gcc-3.3.2 (latest Fedora packages) goes up 160M+ in memory usage,
For a test run I started mozilla with some very long pages, oowriter,
gedit with the listed files opened and some gnome-terminal and a new project
compile with make -j2 so the callbacks.c and interface.c were compiled
simultaneously.

The system is a dual PIII/500 MHz with 512M RAM, not a speedy machine.
With all these, the system used only ~36M swap. Free memory was about 3-6M
then I exited oowriter. Free mem suddenly went up to 75M+, starting
programs was almost instant.

Definitely a win.

-- 
Best regards,
Zoltán Böszörményi

---------------------
What did Hussein say about his knife?
One in Bush worth two in the hand.

Re: [PATCH] Autoregulate vm swappiness cleanup

[Richard B. Johnson]

On Tue, 28 Oct 2003, Boszormenyi Zoltan wrote:
[SNIPPED...]

> -rw-rw-r--    1 zozo     zozo      1090912 okt 27 22:54 interface.c
                                     ^^^^^^^
Guess you use `vim` to edit ...eh?

Linux does have a good linker, you know. You don't need to put
everything in one file!

Cheers,
Dick Johnson
Penguin : Linux version 2.4.22 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.

Re: [PATCH] Autoregulate vm swappiness cleanup

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

On Tue, 28 Oct 2003 09:39:53 EST, "Richard B. Johnson" said:
> On Tue, 28 Oct 2003, Boszormenyi Zoltan wrote:
> [SNIPPED...]
> 
> > -rw-rw-r--    1 zozo     zozo      1090912 okt 27 22:54 interface.c
>                                      ^^^^^^^
> Guess you use `vim` to edit ...eh?
> 
> Linux does have a good linker, you know. You don't need to put
> everything in one file!

On the flip side, if there's a lot of routines all declared 'static' so they are
only visible to that .c file, it's less than simple to split them out and
tell the *rest* of the projects that 'routines in interface/*.c are visible
to each other, but not to C code in database/*.c'.

The Linux kernel has the same issues:

% find . -name '*.[ch]' | xargs grep acpi_bus_unregister_driver

referenced only in drivers/acpi and one include file - but pollutes the global
linkage namespace all the same.

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH] Autoregulate vm swappiness cleanup

[Richard B. Johnson]

On Tue, 28 Oct 2003 Valdis.Kletnieks@vt.edu wrote:

> On Tue, 28 Oct 2003 09:39:53 EST, "Richard B. Johnson" said:
> > On Tue, 28 Oct 2003, Boszormenyi Zoltan wrote:
> > [SNIPPED...]
> >
> > > -rw-rw-r--    1 zozo     zozo      1090912 okt 27 22:54 interface.c
> >                                      ^^^^^^^
> > Guess you use `vim` to edit ...eh?
> >
> > Linux does have a good linker, you know. You don't need to put
> > everything in one file!
>
> On the flip side, if there's a lot of routines all declared 'static' so they are
> only visible to that .c file, it's less than simple to split them out and
> tell the *rest* of the projects that 'routines in interface/*.c are visible
> to each other, but not to C code in database/*.c'.
>
> The Linux kernel has the same issues:
>
> % find . -name '*.[ch]' | xargs grep acpi_bus_unregister_driver
>
> referenced only in drivers/acpi and one include file - but pollutes the global
> linkage namespace all the same.
>

I keep hearing 'pollutes the global namespace' all the time, now. Do
you mean that 'C' code designers actually intend to use the same
name for different procedures or objects in different files?

If so, that's a problem of idiocy, not pollution. Anybody who
writes code for a living would not do that, now would they??

The fact that there may be many global references that need to
be resolved by the linker is not pollution. It's using tools to
simplify design, implementation, testing, and maintenance.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.22 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.

Re: [PATCH] Autoregulate vm swappiness cleanup

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

On Tue, 28 Oct 2003 10:54:56 EST, "Richard B. Johnson" said:

> I keep hearing 'pollutes the global namespace' all the time, now. Do
> you mean that 'C' code designers actually intend to use the same
> name for different procedures or objects in different files?

Right. Exactly.

That's why you need to have module1_debug_level, module2_debug_level,
module3_debug_level and so on, rather than have a 'debug_level' that's visible
in module1, another that's visible in module2, and a third that's visible in
module3. Note that you *CAN* do that if module1, module2, and module3 are all
*single* .c files simply by declaring it 'static int debug_level;'.  But if you
decide to splt module1 and module2 into 2 .c files each, suddenly you need to
rename *both* variables....

And I'm *positive* that *nobody* on this list has done that, forgotten to
rename the variables, and then wondered why module1 has 'debug_level = 1;' in
it and the debugging output starts and then mysteriously stops some random time
after (because module2 has 'debug_level = 0;' in it...)

Yes, C does not have a C++ idea of "private data".  My point is that quite
often, the use of 'static' to simulate it is the reason for very large .c
files....


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH] Autoregulate vm swappiness cleanup

[Måns Rullgård]

Valdis.Kletnieks@vt.edu writes:

> And I'm *positive* that *nobody* on this list has done that,
> forgotten to rename the variables, and then wondered why module1 has
> 'debug_level = 1;' in it and the debugging output starts and then
> mysteriously stops some random time after (because module2 has
> 'debug_level = 0;' in it...)

Doing that mistake would give a linker error.

-- 
Måns Rullgård
mru@kth.se

Re: [PATCH] Autoregulate vm swappiness cleanup

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

On Tue, 28 Oct 2003 17:51:48 +0100, mru@kth.se (=?iso-8859-1?q?M=E5ns_Rullg=E5rd?=)  said:

> Doing that mistake would give a linker error.

Not necessarily.  Consider shared libraries.  Load module1.so, load module2.so,
and then load module3.so that has an unresolved reference that both of the
other two happen to have. Oh wow.. results that are dependent on the order of
-l flags in a non-obvious way...  And '-z muldefs' is always a good way to
shoot yourself in the foot.....

Yes, it's stupid, yes, it's a mistake - my point is that the design of C makes that
sort of mistake more likely.  If there *did* exist a "private to all the pieces of the
module" status similar to the "private to THIS .c 'static'", it wouldn't be anywhere
near the problem.

And don't tell me that the Linux kernel hasn't seen similar issues - a supposedly
private interface that didn't have 'static' because it *was* called from 2 other
.c files in the directory.  And then we have to tell people "don't call that interface,
it's private".

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

[OT] Re: [PATCH] Autoregulate vm swappiness cleanup

[Boszormenyi Zoltan]

Richard B. Johnson írta:
> On Tue, 28 Oct 2003, Boszormenyi Zoltan wrote:
> [SNIPPED...]
> 
> 
>>-rw-rw-r--    1 zozo     zozo      1090912 okt 27 22:54 interface.c
> 
>                                      ^^^^^^^
> Guess you use `vim` to edit ...eh?
> 
> Linux does have a good linker, you know. You don't need to put
> everything in one file!
> 
> Cheers,
> Dick Johnson

Ehm, yes, I know. But I guess you don't use Glade, do you?
interface[.ch] are completely generated by Glade, all the
create_windowXXX() functions are dumped into these files
and created from scratch all the time you save/regenerate
the project's source. The project grew over time, it's
pretty large now.

-- 
Best regards,
Zoltán Böszörményi

---------------------
What did Hussein say about his knife?
One in Bush worth two in the hand.

Re: [PATCH] Autoregulate vm swappiness cleanup

[Con Kolivas]

On Tue, 28 Oct 2003 22:04, Pavel Machek wrote:
> Hi!

Hello!

> I believe swappiness == 100 was "I want max throughput, I don't care
> about latency going through roof", while swappiness == 0 was "I don't
> want you to swap too much, behave reasonably".
>
> As you don't know if user cares about latency or not, I don't see how
> you can autotune this.

Well I guess you either see merit in what my patch does based on what I said, 
or you don't... so I guess you don't. That's fine; I just offered why I felt 
this helped in my varied workloads more than a static value did.

Con

Re: [PATCH] Autoregulate vm swappiness cleanup

[Pavel Machek]

Hi!

> > > It seems that you don't need si_swapinfo here, do you? i.freeram,
> > > i.bufferram, and i.totalram all come from meminfo, as far as I can
> > > see? Maybe I'm missing a bit ...
> >
> > Well I did do it a while ago and it seems I got carried away adding and
> > subtracting info indeed. :-) Here's a simpler patch that does the same
> > thing.
> 
> The off-list enthusiasm has been rather strong so here is a patch done the 
> right way (tm). There is no need for the check of totalram being zero (the 
> original version of this patch modified the swappiness every tick which was 
> wasteful and had a divide by zero on init). Adjusting vm_swappiness only when 
> there is pressure to swap means totalram shouldn't be ever be zero. The 
> sysctl is made read only since writing to it would be ignored now.

I believe swappiness == 100 was "I want max throughput, I don't care
about latency going through roof", while swappiness == 0 was "I don't
want you to swap too much, behave reasonably".

As you don't know if user cares about latency or not, I don't see how
you can autotune this.

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [PATCH] Autoregulate vm swappiness cleanup

[Nick Piggin]

Con Kolivas wrote:

> Nick Piggin wrote:
>
>>
>> Hi Con,
>> If this indeed makes VM behaviour better, why not just merge the 
>> calculation
>> with the swap_tendancy calculation and leave vm_swappiness there as a 
>> tunable?
>
>
> Because the whole point of it is to remove the tunable and make it 
> auto tuning. We could do away with the vm_swappiness variable 
> altogether too (which I would actually prefer to do) but this leaves 
> it intact to see what the vm is doing.


Right. This just had me a bit confused. No worries.

Re: [PATCH] Autoregulate vm swappiness cleanup

[Nick Piggin]

Con Kolivas wrote:

>On Fri, 24 Oct 2003 01:03, Con Kolivas wrote:
>
>>On Friday 24 October 2003 00:42, Martin J. Bligh wrote:
>>
>>>It seems that you don't need si_swapinfo here, do you? i.freeram,
>>>i.bufferram, and i.totalram all come from meminfo, as far as I can
>>>see? Maybe I'm missing a bit ...
>>>
>>Well I did do it a while ago and it seems I got carried away adding and
>>subtracting info indeed. :-) Here's a simpler patch that does the same
>>thing.
>>
>
>The off-list enthusiasm has been rather strong so here is a patch done the 
>right way (tm). There is no need for the check of totalram being zero (the 
>original version of this patch modified the swappiness every tick which was 
>wasteful and had a divide by zero on init). Adjusting vm_swappiness only when 
>there is pressure to swap means totalram shouldn't be ever be zero. The 
>sysctl is made read only since writing to it would be ignored now.
>
>Con
>
>
>
>------------------------------------------------------------------------
>
>--- linux-2.6.0-test8-base/kernel/sysctl.c	2003-10-19 20:24:49.000000000 +1000
>+++ linux-2.6.0-test8-am/kernel/sysctl.c	2003-10-25 16:37:44.384824976 +1000
>@@ -664,11 +664,8 @@ static ctl_table vm_table[] = {
> 		.procname	= "swappiness",
> 		.data		= &vm_swappiness,
> 		.maxlen		= sizeof(vm_swappiness),
>-		.mode		= 0644,
>-		.proc_handler	= &proc_dointvec_minmax,
>-		.strategy	= &sysctl_intvec,
>-		.extra1		= &zero,
>-		.extra2		= &one_hundred,
>+		.mode		= 0444 /* read-only*/,
>+		.proc_handler	= &proc_dointvec,
> 	},
> #ifdef CONFIG_HUGETLB_PAGE
> 	 {
>--- linux-2.6.0-test8-base/mm/vmscan.c	2003-10-19 20:24:36.000000000 +1000
>+++ linux-2.6.0-test8-am/mm/vmscan.c	2003-10-25 16:40:33.099176496 +1000
>@@ -47,7 +47,7 @@
> /*
>  * From 0 .. 100.  Higher means more swappy.
>  */
>-int vm_swappiness = 60;
>+int vm_swappiness = 0;
> static long total_memory;
> 
> #ifdef ARCH_HAS_PREFETCH
>@@ -600,6 +600,7 @@ refill_inactive_zone(struct zone *zone, 
> 	LIST_HEAD(l_active);	/* Pages to go onto the active_list */
> 	struct page *page;
> 	struct pagevec pvec;
>+	struct sysinfo i;
> 	int reclaim_mapped = 0;
> 	long mapped_ratio;
> 	long distress;
>@@ -642,6 +643,14 @@ refill_inactive_zone(struct zone *zone, 
> 	mapped_ratio = (ps->nr_mapped * 100) / total_memory;
> 
> 	/*
>+	 * Autoregulate vm_swappiness to be equal to the percentage of
>+	 * pages in physical ram that are application pages. -ck
>+	 */
>+	si_meminfo(&i);
>+	vm_swappiness = 100 - (((i.freeram + get_page_cache_size() -
>+		swapper_space.nrpages) * 100) / i.totalram);
>+
>+	/*
> 	 * Now decide how much we really want to unmap some pages.  The mapped
> 	 * ratio is downgraded - just because there's a lot of mapped memory
> 	 * doesn't necessarily mean that page reclaim isn't succeeding.
>

Hi Con,
If this indeed makes VM behaviour better, why not just merge the calculation
with the swap_tendancy calculation and leave vm_swappiness there as a 
tunable?

Re: [PATCH] Autoregulate vm swappiness cleanup

[Con Kolivas]

Nick Piggin wrote:
> 
> Hi Con,
> If this indeed makes VM behaviour better, why not just merge the 
> calculation
> with the swap_tendancy calculation and leave vm_swappiness there as a 
> tunable?

Because the whole point of it is to remove the tunable and make it auto 
tuning. We could do away with the vm_swappiness variable altogether too 
(which I would actually prefer to do) but this leaves it intact to see 
what the vm is doing.

Con

[PATCH] Autoregulate vm swappiness cleanup

[Con Kolivas]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

On Fri, 24 Oct 2003 01:03, Con Kolivas wrote:
> On Friday 24 October 2003 00:42, Martin J. Bligh wrote:
> > It seems that you don't need si_swapinfo here, do you? i.freeram,
> > i.bufferram, and i.totalram all come from meminfo, as far as I can
> > see? Maybe I'm missing a bit ...
>
> Well I did do it a while ago and it seems I got carried away adding and
> subtracting info indeed. :-) Here's a simpler patch that does the same
> thing.

The off-list enthusiasm has been rather strong so here is a patch done the 
right way (tm). There is no need for the check of totalram being zero (the 
original version of this patch modified the swappiness every tick which was 
wasteful and had a divide by zero on init). Adjusting vm_swappiness only when 
there is pressure to swap means totalram shouldn't be ever be zero. The 
sysctl is made read only since writing to it would be ignored now.

Con

[-- Attachment #2: patch-test8-am-3 --]
[-- Type: text/x-diff, Size: 1725 bytes --]

--- linux-2.6.0-test8-base/kernel/sysctl.c	2003-10-19 20:24:49.000000000 +1000
+++ linux-2.6.0-test8-am/kernel/sysctl.c	2003-10-25 16:37:44.384824976 +1000
@@ -664,11 +664,8 @@ static ctl_table vm_table[] = {
 		.procname	= "swappiness",
 		.data		= &vm_swappiness,
 		.maxlen		= sizeof(vm_swappiness),
-		.mode		= 0644,
-		.proc_handler	= &proc_dointvec_minmax,
-		.strategy	= &sysctl_intvec,
-		.extra1		= &zero,
-		.extra2		= &one_hundred,
+		.mode		= 0444 /* read-only*/,
+		.proc_handler	= &proc_dointvec,
 	},
 #ifdef CONFIG_HUGETLB_PAGE
 	 {
--- linux-2.6.0-test8-base/mm/vmscan.c	2003-10-19 20:24:36.000000000 +1000
+++ linux-2.6.0-test8-am/mm/vmscan.c	2003-10-25 16:40:33.099176496 +1000
@@ -47,7 +47,7 @@
 /*
  * From 0 .. 100.  Higher means more swappy.
  */
-int vm_swappiness = 60;
+int vm_swappiness = 0;
 static long total_memory;
 
 #ifdef ARCH_HAS_PREFETCH
@@ -600,6 +600,7 @@ refill_inactive_zone(struct zone *zone, 
 	LIST_HEAD(l_active);	/* Pages to go onto the active_list */
 	struct page *page;
 	struct pagevec pvec;
+	struct sysinfo i;
 	int reclaim_mapped = 0;
 	long mapped_ratio;
 	long distress;
@@ -642,6 +643,14 @@ refill_inactive_zone(struct zone *zone, 
 	mapped_ratio = (ps->nr_mapped * 100) / total_memory;
 
 	/*
+	 * Autoregulate vm_swappiness to be equal to the percentage of
+	 * pages in physical ram that are application pages. -ck
+	 */
+	si_meminfo(&i);
+	vm_swappiness = 100 - (((i.freeram + get_page_cache_size() -
+		swapper_space.nrpages) * 100) / i.totalram);
+
+	/*
 	 * Now decide how much we really want to unmap some pages.  The mapped
 	 * ratio is downgraded - just because there's a lot of mapped memory
 	 * doesn't necessarily mean that page reclaim isn't succeeding.