2014-02-07 17:41:32

by Johannes Weiner

[permalink] [raw]
Subject: [patch] drop_caches: add some documentation and info message

From: Dave Hansen <[email protected]>

There is plenty of anecdotal evidence and a load of blog posts
suggesting that using "drop_caches" periodically keeps your system
running in "tip top shape". Perhaps adding some kernel documentation
will increase the amount of accurate data on its use.

If we are not shrinking caches effectively, then we have real bugs.
Using drop_caches will simply mask the bugs and make them harder to
find, but certainly does not fix them, nor is it an appropriate
"workaround" to limit the size of the caches. On the contrary, there
have been bug reports on issues that turned out to be misguided use of
cache dropping.

Dropping caches is a very drastic and disruptive operation that is
good for debugging and running tests, but if it creates bug reports
from production use, kernel developers should be aware of its use.

Add a bit more documentation about it, and add a little KERN_NOTICE.

[[email protected]: checkpatch fixes]
Signed-off-by: Dave Hansen <[email protected]>
Signed-off-by: Michal Hocko <[email protected]>
Acked-by: KOSAKI Motohiro <[email protected]>
Acked-by: KAMEZAWA Hiroyuki <[email protected]>
Signed-off-by: Johannes Weiner <[email protected]>
---
Documentation/sysctl/vm.txt | 33 +++++++++++++++++++++++++++------
fs/drop_caches.c | 4 ++++
2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
index d614a9b6a280..36278c610a5f 100644
--- a/Documentation/sysctl/vm.txt
+++ b/Documentation/sysctl/vm.txt
@@ -175,18 +175,39 @@ Setting this to zero disables periodic writeback altogether.

drop_caches

-Writing to this will cause the kernel to drop clean caches, dentries and
-inodes from memory, causing that memory to become free.
+Writing to this will cause the kernel to drop clean caches, as well as
+reclaimable slab objects like dentries and inodes. Once dropped, their
+memory becomes free.

To free pagecache:
echo 1 > /proc/sys/vm/drop_caches
-To free dentries and inodes:
+To free reclaimable slab objects (includes dentries and inodes):
echo 2 > /proc/sys/vm/drop_caches
-To free pagecache, dentries and inodes:
+To free slab objects and pagecache:
echo 3 > /proc/sys/vm/drop_caches

-As this is a non-destructive operation and dirty objects are not freeable, the
-user should run `sync' first.
+This is a non-destructive operation and will not free any dirty objects.
+To increase the number of objects freed by this operation, the user may run
+`sync' prior to writing to /proc/sys/vm/drop_caches. This will minimize the
+number of dirty objects on the system and create more candidates to be
+dropped.
+
+This file is not a means to control the growth of the various kernel caches
+(inodes, dentries, pagecache, etc...) These objects are automatically
+reclaimed by the kernel when memory is needed elsewhere on the system.
+
+Use of this file can cause performance problems. Since it discards cached
+objects, it may cost a significant amount of I/O and CPU to recreate the
+dropped objects, especially if they were under heavy use. Because of this,
+use outside of a testing or debugging environment is not recommended.
+
+You may see informational messages in your kernel log when this file is
+used:
+
+ cat (1234): dropped kernel caches: 3
+
+These are informational only. They do not mean that anything is wrong
+with your system.

==============================================================

diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 9fd702f5bfb2..3579d391e950 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -5,6 +5,7 @@
#include <linux/kernel.h>
#include <linux/mm.h>
#include <linux/fs.h>
+#include <linux/ratelimit.h>
#include <linux/writeback.h>
#include <linux/sysctl.h>
#include <linux/gfp.h>
@@ -59,6 +60,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
if (ret)
return ret;
if (write) {
+ printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
+ current->comm, task_pid_nr(current),
+ sysctl_drop_caches);
if (sysctl_drop_caches & 1)
iterate_supers(drop_pagecache_sb, NULL);
if (sysctl_drop_caches & 2)
--
1.8.5.3


2014-02-07 17:57:29

by Rik van Riel

[permalink] [raw]
Subject: Re: [patch] drop_caches: add some documentation and info message

On 02/07/2014 12:40 PM, Johannes Weiner wrote:

> @@ -59,6 +60,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> if (ret)
> return ret;
> if (write) {
> + printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> + current->comm, task_pid_nr(current),
> + sysctl_drop_caches);
> if (sysctl_drop_caches & 1)
> iterate_supers(drop_pagecache_sb, NULL);
> if (sysctl_drop_caches & 2)
>

Would it be better to print this after the operation
has completed?

2014-02-07 18:13:48

by Johannes Weiner

[permalink] [raw]
Subject: Re: [patch] drop_caches: add some documentation and info message

On Fri, Feb 07, 2014 at 12:55:37PM -0500, Rik van Riel wrote:
> On 02/07/2014 12:40 PM, Johannes Weiner wrote:
>
> >@@ -59,6 +60,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> > if (ret)
> > return ret;
> > if (write) {
> >+ printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> >+ current->comm, task_pid_nr(current),
> >+ sysctl_drop_caches);
> > if (sysctl_drop_caches & 1)
> > iterate_supers(drop_pagecache_sb, NULL);
> > if (sysctl_drop_caches & 2)
> >
>
> Would it be better to print this after the operation
> has completed?

It would make more sense grammatically :-) Either way is fine with me,
updated below to inform after the fact.

---
From: Dave Hansen <[email protected]>
Date: Fri, 12 Oct 2012 14:30:54 +0200
Subject: [patch] drop_caches: add some documentation and info message

There is plenty of anecdotal evidence and a load of blog posts
suggesting that using "drop_caches" periodically keeps your system
running in "tip top shape". Perhaps adding some kernel documentation
will increase the amount of accurate data on its use.

If we are not shrinking caches effectively, then we have real bugs.
Using drop_caches will simply mask the bugs and make them harder to
find, but certainly does not fix them, nor is it an appropriate
"workaround" to limit the size of the caches. On the contrary, there
have been bug reports on issues that turned out to be misguided use of
cache dropping.

Dropping caches is a very drastic and disruptive operation that is
good for debugging and running tests, but if it creates bug reports
from production use, kernel developers should be aware of its use.

Add a bit more documentation about it, and add a little KERN_NOTICE.

[[email protected]: checkpatch fixes]
Signed-off-by: Dave Hansen <[email protected]>
Signed-off-by: Michal Hocko <[email protected]>
Acked-by: KOSAKI Motohiro <[email protected]>
Acked-by: KAMEZAWA Hiroyuki <[email protected]>
Signed-off-by: Johannes Weiner <[email protected]>
---
Documentation/sysctl/vm.txt | 33 +++++++++++++++++++++++++++------
fs/drop_caches.c | 4 ++++
2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
index d614a9b6a280..36278c610a5f 100644
--- a/Documentation/sysctl/vm.txt
+++ b/Documentation/sysctl/vm.txt
@@ -175,18 +175,39 @@ Setting this to zero disables periodic writeback altogether.

drop_caches

-Writing to this will cause the kernel to drop clean caches, dentries and
-inodes from memory, causing that memory to become free.
+Writing to this will cause the kernel to drop clean caches, as well as
+reclaimable slab objects like dentries and inodes. Once dropped, their
+memory becomes free.

To free pagecache:
echo 1 > /proc/sys/vm/drop_caches
-To free dentries and inodes:
+To free reclaimable slab objects (includes dentries and inodes):
echo 2 > /proc/sys/vm/drop_caches
-To free pagecache, dentries and inodes:
+To free slab objects and pagecache:
echo 3 > /proc/sys/vm/drop_caches

-As this is a non-destructive operation and dirty objects are not freeable, the
-user should run `sync' first.
+This is a non-destructive operation and will not free any dirty objects.
+To increase the number of objects freed by this operation, the user may run
+`sync' prior to writing to /proc/sys/vm/drop_caches. This will minimize the
+number of dirty objects on the system and create more candidates to be
+dropped.
+
+This file is not a means to control the growth of the various kernel caches
+(inodes, dentries, pagecache, etc...) These objects are automatically
+reclaimed by the kernel when memory is needed elsewhere on the system.
+
+Use of this file can cause performance problems. Since it discards cached
+objects, it may cost a significant amount of I/O and CPU to recreate the
+dropped objects, especially if they were under heavy use. Because of this,
+use outside of a testing or debugging environment is not recommended.
+
+You may see informational messages in your kernel log when this file is
+used:
+
+ cat (1234): dropped kernel caches: 3
+
+These are informational only. They do not mean that anything is wrong
+with your system.

==============================================================

diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 9fd702f5bfb2..02ae3386e08f 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -5,6 +5,7 @@
#include <linux/kernel.h>
#include <linux/mm.h>
#include <linux/fs.h>
+#include <linux/ratelimit.h>
#include <linux/writeback.h>
#include <linux/sysctl.h>
#include <linux/gfp.h>
@@ -63,6 +64,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
iterate_supers(drop_pagecache_sb, NULL);
if (sysctl_drop_caches & 2)
drop_slab();
+ printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
+ current->comm, task_pid_nr(current),
+ sysctl_drop_caches);
}
return 0;
}
--
1.8.5.3

2014-02-07 19:30:59

by Rik van Riel

[permalink] [raw]
Subject: Re: [patch] drop_caches: add some documentation and info message

On 02/07/2014 01:13 PM, Johannes Weiner wrote:

>> Would it be better to print this after the operation
>> has completed?
>
> It would make more sense grammatically :-) Either way is fine with me,
> updated below to inform after the fact.

Well, in principle the operation could take an arbitrarily
long time, so there are some minor concerns besides
grammatical correctness, too :)

Thanks for updating the patch.

> ---
> From: Dave Hansen <[email protected]>
> Date: Fri, 12 Oct 2012 14:30:54 +0200
> Subject: [patch] drop_caches: add some documentation and info message

> Dropping caches is a very drastic and disruptive operation that is
> good for debugging and running tests, but if it creates bug reports
> from production use, kernel developers should be aware of its use.
>
> Add a bit more documentation about it, and add a little KERN_NOTICE.
>
> [[email protected]: checkpatch fixes]
> Signed-off-by: Dave Hansen <[email protected]>
> Signed-off-by: Michal Hocko <[email protected]>
> Acked-by: KOSAKI Motohiro <[email protected]>
> Acked-by: KAMEZAWA Hiroyuki <[email protected]>
> Signed-off-by: Johannes Weiner <[email protected]>

Reviewed-by: Rik van Riel <[email protected]>

2014-02-07 20:31:32

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] drop_caches: add some documentation and info message

On Fri, 7 Feb 2014 13:13:32 -0500 Johannes Weiner <[email protected]> wrote:

> @@ -63,6 +64,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> iterate_supers(drop_pagecache_sb, NULL);
> if (sysctl_drop_caches & 2)
> drop_slab();
> + printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> + current->comm, task_pid_nr(current),
> + sysctl_drop_caches);
> }
> return 0;
> }

My concern with this is that there may be people whose
other-party-provided software uses drop_caches. Their machines will
now sit there emitting log messages and there's nothing they can do
about it, apart from whining at their vendors.


We could do something like this?

--- a/fs/drop_caches.c~drop_caches-add-some-documentation-and-info-message-fix
+++ a/fs/drop_caches.c
@@ -60,13 +60,17 @@ int drop_caches_sysctl_handler(ctl_table
if (ret)
return ret;
if (write) {
+ static int stfu;
+
if (sysctl_drop_caches & 1)
iterate_supers(drop_pagecache_sb, NULL);
if (sysctl_drop_caches & 2)
drop_slab();
- printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
- current->comm, task_pid_nr(current),
- sysctl_drop_caches);
+ stfu |= sysctl_drop_caches & 4;
+ if (!stfu)
+ pr_info_ratelimited("%s (%d): dropped kernel caches: %d\n",
+ current->comm, task_pid_nr(current),
+ sysctl_drop_caches);
}
return 0;
}
_

(note switch to pr_info_ratelimited)

2014-02-07 21:26:16

by Johannes Weiner

[permalink] [raw]
Subject: Re: [patch] drop_caches: add some documentation and info message

On Fri, Feb 07, 2014 at 12:31:29PM -0800, Andrew Morton wrote:
> On Fri, 7 Feb 2014 13:13:32 -0500 Johannes Weiner <[email protected]> wrote:
>
> > @@ -63,6 +64,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> > iterate_supers(drop_pagecache_sb, NULL);
> > if (sysctl_drop_caches & 2)
> > drop_slab();
> > + printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> > + current->comm, task_pid_nr(current),
> > + sysctl_drop_caches);
> > }
> > return 0;
> > }
>
> My concern with this is that there may be people whose
> other-party-provided software uses drop_caches. Their machines will
> now sit there emitting log messages and there's nothing they can do
> about it, apart from whining at their vendors.

Ironically, we have a customer that is complaining that we currently
do not log these events, and they want to know who in their stack is
being idiotic.

> We could do something like this?

They can already change the log level. The below will suppress
valuable debugging information in a way that still results in
inconspicuous looking syslog excerpts, which somewhat undermines the
original motivation for this change.

So I'm not fond of it, but I'd rather have this patch with it than no
patch at all. As long as the message is printed per default.

> --- a/fs/drop_caches.c~drop_caches-add-some-documentation-and-info-message-fix
> +++ a/fs/drop_caches.c
> @@ -60,13 +60,17 @@ int drop_caches_sysctl_handler(ctl_table
> if (ret)
> return ret;
> if (write) {
> + static int stfu;
> +
> if (sysctl_drop_caches & 1)
> iterate_supers(drop_pagecache_sb, NULL);
> if (sysctl_drop_caches & 2)
> drop_slab();
> - printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> - current->comm, task_pid_nr(current),
> - sysctl_drop_caches);
> + stfu |= sysctl_drop_caches & 4;
> + if (!stfu)
> + pr_info_ratelimited("%s (%d): dropped kernel caches: %d\n",
> + current->comm, task_pid_nr(current),
> + sysctl_drop_caches);
> }
> return 0;
> }
> _
>
> (note switch to pr_info_ratelimited)

2014-02-08 11:56:15

by Rafael Aquini

[permalink] [raw]
Subject: Re: [patch] drop_caches: add some documentation and info message

On Fri, Feb 07, 2014 at 01:13:32PM -0500, Johannes Weiner wrote:
> On Fri, Feb 07, 2014 at 12:55:37PM -0500, Rik van Riel wrote:
> > On 02/07/2014 12:40 PM, Johannes Weiner wrote:
> >
> > >@@ -59,6 +60,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> > > if (ret)
> > > return ret;
> > > if (write) {
> > >+ printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> > >+ current->comm, task_pid_nr(current),
> > >+ sysctl_drop_caches);
> > > if (sysctl_drop_caches & 1)
> > > iterate_supers(drop_pagecache_sb, NULL);
> > > if (sysctl_drop_caches & 2)
> > >
> >
> > Would it be better to print this after the operation
> > has completed?
>
> It would make more sense grammatically :-) Either way is fine with me,
> updated below to inform after the fact.
>
> ---
> From: Dave Hansen <[email protected]>
> Date: Fri, 12 Oct 2012 14:30:54 +0200
> Subject: [patch] drop_caches: add some documentation and info message
>
> There is plenty of anecdotal evidence and a load of blog posts
> suggesting that using "drop_caches" periodically keeps your system
> running in "tip top shape". Perhaps adding some kernel documentation
> will increase the amount of accurate data on its use.
>
> If we are not shrinking caches effectively, then we have real bugs.
> Using drop_caches will simply mask the bugs and make them harder to
> find, but certainly does not fix them, nor is it an appropriate
> "workaround" to limit the size of the caches. On the contrary, there
> have been bug reports on issues that turned out to be misguided use of
> cache dropping.
>
> Dropping caches is a very drastic and disruptive operation that is
> good for debugging and running tests, but if it creates bug reports
> from production use, kernel developers should be aware of its use.
>
> Add a bit more documentation about it, and add a little KERN_NOTICE.
>
> [[email protected]: checkpatch fixes]
> Signed-off-by: Dave Hansen <[email protected]>
> Signed-off-by: Michal Hocko <[email protected]>
> Acked-by: KOSAKI Motohiro <[email protected]>
> Acked-by: KAMEZAWA Hiroyuki <[email protected]>
> Signed-off-by: Johannes Weiner <[email protected]>
> ---
> Documentation/sysctl/vm.txt | 33 +++++++++++++++++++++++++++------
> fs/drop_caches.c | 4 ++++
> 2 files changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
> index d614a9b6a280..36278c610a5f 100644
> --- a/Documentation/sysctl/vm.txt
> +++ b/Documentation/sysctl/vm.txt
> @@ -175,18 +175,39 @@ Setting this to zero disables periodic writeback altogether.
>
> drop_caches
>
> -Writing to this will cause the kernel to drop clean caches, dentries and
> -inodes from memory, causing that memory to become free.
> +Writing to this will cause the kernel to drop clean caches, as well as
> +reclaimable slab objects like dentries and inodes. Once dropped, their
> +memory becomes free.
>
> To free pagecache:
> echo 1 > /proc/sys/vm/drop_caches
> -To free dentries and inodes:
> +To free reclaimable slab objects (includes dentries and inodes):
> echo 2 > /proc/sys/vm/drop_caches
> -To free pagecache, dentries and inodes:
> +To free slab objects and pagecache:
> echo 3 > /proc/sys/vm/drop_caches
>
> -As this is a non-destructive operation and dirty objects are not freeable, the
> -user should run `sync' first.
> +This is a non-destructive operation and will not free any dirty objects.
> +To increase the number of objects freed by this operation, the user may run
> +`sync' prior to writing to /proc/sys/vm/drop_caches. This will minimize the
> +number of dirty objects on the system and create more candidates to be
> +dropped.
> +
> +This file is not a means to control the growth of the various kernel caches
> +(inodes, dentries, pagecache, etc...) These objects are automatically
> +reclaimed by the kernel when memory is needed elsewhere on the system.
> +
> +Use of this file can cause performance problems. Since it discards cached
> +objects, it may cost a significant amount of I/O and CPU to recreate the
> +dropped objects, especially if they were under heavy use. Because of this,
> +use outside of a testing or debugging environment is not recommended.
> +
> +You may see informational messages in your kernel log when this file is
> +used:
> +
> + cat (1234): dropped kernel caches: 3
> +
> +These are informational only. They do not mean that anything is wrong
> +with your system.
>
> ==============================================================
>
> diff --git a/fs/drop_caches.c b/fs/drop_caches.c
> index 9fd702f5bfb2..02ae3386e08f 100644
> --- a/fs/drop_caches.c
> +++ b/fs/drop_caches.c
> @@ -5,6 +5,7 @@
> #include <linux/kernel.h>
> #include <linux/mm.h>
> #include <linux/fs.h>
> +#include <linux/ratelimit.h>
> #include <linux/writeback.h>
> #include <linux/sysctl.h>
> #include <linux/gfp.h>
> @@ -63,6 +64,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> iterate_supers(drop_pagecache_sb, NULL);
> if (sysctl_drop_caches & 2)
> drop_slab();
> + printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",

Just a nitpick here:
s/printk_ratelimited(KERN_INFO ...)/pr_info_ratelimited(...)


Acked-by: Rafael Aquini <[email protected]>


> + current->comm, task_pid_nr(current),
> + sysctl_drop_caches);
> }
> return 0;
> }
> --
> 1.8.5.3
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

2014-02-10 20:51:10

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] drop_caches: add some documentation and info message

On Fri, 7 Feb 2014 16:26:01 -0500 Johannes Weiner <[email protected]> wrote:

> On Fri, Feb 07, 2014 at 12:31:29PM -0800, Andrew Morton wrote:
> > On Fri, 7 Feb 2014 13:13:32 -0500 Johannes Weiner <[email protected]> wrote:
> >
> > > @@ -63,6 +64,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> > > iterate_supers(drop_pagecache_sb, NULL);
> > > if (sysctl_drop_caches & 2)
> > > drop_slab();
> > > + printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> > > + current->comm, task_pid_nr(current),
> > > + sysctl_drop_caches);
> > > }
> > > return 0;
> > > }
> >
> > My concern with this is that there may be people whose
> > other-party-provided software uses drop_caches. Their machines will
> > now sit there emitting log messages and there's nothing they can do
> > about it, apart from whining at their vendors.
>
> Ironically, we have a customer that is complaining that we currently
> do not log these events, and they want to know who in their stack is
> being idiotic.

Right. But if we release a kernel which goes blah on every write to
drop_caches, that customer has logs full of blahs which they are
now totally uninterested in.

> > We could do something like this?
>
> They can already change the log level.

Suppressing unrelated things...

> The below will suppress
> valuable debugging information in a way that still results in
> inconspicuous looking syslog excerpts, which somewhat undermines the
> original motivation for this change.

Yes, somewhat. It is a compromise. You can see my concern here?

2014-02-10 21:12:18

by Motohiro Kosaki

[permalink] [raw]
Subject: RE: [patch] drop_caches: add some documentation and info message



> -----Original Message-----
> From: Andrew Morton [mailto:[email protected]]
> Sent: Tuesday, February 11, 2014 5:51 AM
> To: Johannes Weiner
> Cc: Rik van Riel; Dave Hansen; Michal Hocko; Motohiro Kosaki JP; KAMEZAWA
> Hiroyuki; [email protected]; [email protected]
> Subject: Re: [patch] drop_caches: add some documentation and info
> message
>
> On Fri, 7 Feb 2014 16:26:01 -0500 Johannes Weiner <[email protected]>
> wrote:
>
> > On Fri, Feb 07, 2014 at 12:31:29PM -0800, Andrew Morton wrote:
> > > On Fri, 7 Feb 2014 13:13:32 -0500 Johannes Weiner
> <[email protected]> wrote:
> > >
> > > > @@ -63,6 +64,9 @@ int drop_caches_sysctl_handler(ctl_table *table,
> int write,
> > > > iterate_supers(drop_pagecache_sb, NULL);
> > > > if (sysctl_drop_caches & 2)
> > > > drop_slab();
> > > > + printk_ratelimited(KERN_INFO "%s (%d): dropped kernel
> caches: %d\n",
> > > > + current->comm, task_pid_nr(current),
> > > > + sysctl_drop_caches);
> > > > }
> > > > return 0;
> > > > }
> > >
> > > My concern with this is that there may be people whose
> > > other-party-provided software uses drop_caches. Their machines will
> > > now sit there emitting log messages and there's nothing they can do
> > > about it, apart from whining at their vendors.
> >
> > Ironically, we have a customer that is complaining that we currently
> > do not log these events, and they want to know who in their stack is
> > being idiotic.
>
> Right. But if we release a kernel which goes blah on every write to
> drop_caches, that customer has logs full of blahs which they are now totally
> uninterested in.

Please let me know if I misunderstand something. This patch uses KERN_INFO.
Then, any log shouldn't be emitted by default.

Moreover, if someone change syslog log level to INFO, they are going to see
much prenty annoying and too much logs even if we reject this patch.



2014-02-10 21:54:44

by Johannes Weiner

[permalink] [raw]
Subject: Re: [patch] drop_caches: add some documentation and info message

On Mon, Feb 10, 2014 at 12:51:02PM -0800, Andrew Morton wrote:
> On Fri, 7 Feb 2014 16:26:01 -0500 Johannes Weiner <[email protected]> wrote:
>
> > On Fri, Feb 07, 2014 at 12:31:29PM -0800, Andrew Morton wrote:
> > > On Fri, 7 Feb 2014 13:13:32 -0500 Johannes Weiner <[email protected]> wrote:
> > >
> > > > @@ -63,6 +64,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> > > > iterate_supers(drop_pagecache_sb, NULL);
> > > > if (sysctl_drop_caches & 2)
> > > > drop_slab();
> > > > + printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> > > > + current->comm, task_pid_nr(current),
> > > > + sysctl_drop_caches);
> > > > }
> > > > return 0;
> > > > }
> > >
> > > My concern with this is that there may be people whose
> > > other-party-provided software uses drop_caches. Their machines will
> > > now sit there emitting log messages and there's nothing they can do
> > > about it, apart from whining at their vendors.
> >
> > Ironically, we have a customer that is complaining that we currently
> > do not log these events, and they want to know who in their stack is
> > being idiotic.
>
> Right. But if we release a kernel which goes blah on every write to
> drop_caches, that customer has logs full of blahs which they are
> now totally uninterested in.
>
> > > We could do something like this?
> >
> > They can already change the log level.
>
> Suppressing unrelated things...
>
> > The below will suppress
> > valuable debugging information in a way that still results in
> > inconspicuous looking syslog excerpts, which somewhat undermines the
> > original motivation for this change.
>
> Yes, somewhat. It is a compromise. You can see my concern here?

How about this: we allow disabling the log message, but print the line
of the disabling call so it's clear who dunnit. To make sure valuable
info is not missing in bug reports, add counters for the two events in
/proc/vmstat.

Does that sound acceptable?

---
From: Dave Hansen <[email protected]>
Date: Fri, 12 Oct 2012 14:30:54 +0200
Subject: [patch] drop_caches: add some documentation and info message

There is plenty of anecdotal evidence and a load of blog posts
suggesting that using "drop_caches" periodically keeps your system
running in "tip top shape". Perhaps adding some kernel documentation
will increase the amount of accurate data on its use.

If we are not shrinking caches effectively, then we have real bugs.
Using drop_caches will simply mask the bugs and make them harder to
find, but certainly does not fix them, nor is it an appropriate
"workaround" to limit the size of the caches. On the contrary, there
have been bug reports on issues that turned out to be misguided use of
cache dropping.

Dropping caches is a very drastic and disruptive operation that is
good for debugging and running tests, but if it creates bug reports
from production use, kernel developers should be aware of its use.

Add a bit more documentation about it, a syslog message to track down
abusers, and vmstat drop counters to help analyze problem reports.

[[email protected]: checkpatch fixes]
Signed-off-by: Dave Hansen <[email protected]>
Signed-off-by: Michal Hocko <[email protected]>
Acked-by: KOSAKI Motohiro <[email protected]>
Acked-by: KAMEZAWA Hiroyuki <[email protected]>
Signed-off-by: Johannes Weiner <[email protected]>
---
Documentation/sysctl/vm.txt | 33 +++++++++++++++++++++++++++------
fs/drop_caches.c | 16 ++++++++++++++--
include/linux/vm_event_item.h | 1 +
kernel/sysctl.c | 4 ++--
mm/vmstat.c | 3 +++
5 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
index d614a9b6a280..e7e544bc4ead 100644
--- a/Documentation/sysctl/vm.txt
+++ b/Documentation/sysctl/vm.txt
@@ -175,18 +175,39 @@ Setting this to zero disables periodic writeback altogether.

drop_caches

-Writing to this will cause the kernel to drop clean caches, dentries and
-inodes from memory, causing that memory to become free.
+Writing to this will cause the kernel to drop clean caches, as well as
+reclaimable slab objects like dentries and inodes. Once dropped, their
+memory becomes free.

To free pagecache:
echo 1 > /proc/sys/vm/drop_caches
-To free dentries and inodes:
+To free reclaimable slab objects (includes dentries and inodes):
echo 2 > /proc/sys/vm/drop_caches
-To free pagecache, dentries and inodes:
+To free slab objects and pagecache:
echo 3 > /proc/sys/vm/drop_caches

-As this is a non-destructive operation and dirty objects are not freeable, the
-user should run `sync' first.
+This is a non-destructive operation and will not free any dirty objects.
+To increase the number of objects freed by this operation, the user may run
+`sync' prior to writing to /proc/sys/vm/drop_caches. This will minimize the
+number of dirty objects on the system and create more candidates to be
+dropped.
+
+This file is not a means to control the growth of the various kernel caches
+(inodes, dentries, pagecache, etc...) These objects are automatically
+reclaimed by the kernel when memory is needed elsewhere on the system.
+
+Use of this file can cause performance problems. Since it discards cached
+objects, it may cost a significant amount of I/O and CPU to recreate the
+dropped objects, especially if they were under heavy use. Because of this,
+use outside of a testing or debugging environment is not recommended.
+
+You may see informational messages in your kernel log when this file is
+used:
+
+ cat (1234): drop_caches: 3
+
+These are informational only. They do not mean that anything is wrong
+with your system. To disable them, echo 4 (bit 3) into drop_caches.

==============================================================

diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 9fd702f5bfb2..9280202e488c 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -59,10 +59,22 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
if (ret)
return ret;
if (write) {
- if (sysctl_drop_caches & 1)
+ static int stfu;
+
+ if (sysctl_drop_caches & 1) {
iterate_supers(drop_pagecache_sb, NULL);
- if (sysctl_drop_caches & 2)
+ count_vm_event(DROP_PAGECACHE);
+ }
+ if (sysctl_drop_caches & 2) {
drop_slab();
+ count_vm_event(DROP_SLAB);
+ }
+ if (!stfu) {
+ pr_info("%s (%d): drop_caches: %d\n",
+ current->comm, task_pid_nr(current),
+ sysctl_drop_caches);
+ }
+ stfu |= sysctl_drop_caches & 4;
}
return 0;
}
diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
index c557c6d096de..99a7d0e71f8c 100644
--- a/include/linux/vm_event_item.h
+++ b/include/linux/vm_event_item.h
@@ -37,6 +37,7 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
PGINODESTEAL, SLABS_SCANNED, KSWAPD_INODESTEAL,
KSWAPD_LOW_WMARK_HIT_QUICKLY, KSWAPD_HIGH_WMARK_HIT_QUICKLY,
PAGEOUTRUN, ALLOCSTALL, PGROTATED,
+ DROP_PAGECACHE, DROP_SLAB,
#ifdef CONFIG_NUMA_BALANCING
NUMA_PTE_UPDATES,
NUMA_HUGE_PTE_UPDATES,
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 49e13e1f8fe6..4b0e0857b4b8 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -126,7 +126,7 @@ static int __maybe_unused neg_one = -1;
static int zero;
static int __maybe_unused one = 1;
static int __maybe_unused two = 2;
-static int __maybe_unused three = 3;
+static int __maybe_unused four = 4;
static unsigned long one_ul = 1;
static int one_hundred = 100;
#ifdef CONFIG_PRINTK
@@ -1283,7 +1283,7 @@ static struct ctl_table vm_table[] = {
.mode = 0644,
.proc_handler = drop_caches_sysctl_handler,
.extra1 = &one,
- .extra2 = &three,
+ .extra2 = &four,
},
#ifdef CONFIG_COMPACTION
{
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 72496140ac08..7887588ce8f8 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -810,6 +810,9 @@ const char * const vmstat_text[] = {

"pgrotated",

+ "drop_pagecache",
+ "drop_slab",
+
#ifdef CONFIG_NUMA_BALANCING
"numa_pte_updates",
"numa_huge_pte_updates",
--
1.8.5.3

2014-02-11 21:45:04

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] drop_caches: add some documentation and info message

On Mon, 10 Feb 2014 16:54:16 -0500 Johannes Weiner <[email protected]> wrote:

> How about this: we allow disabling the log message, but print the line
> of the disabling call so it's clear who dunnit. To make sure valuable
> info is not missing in bug reports, add counters for the two events in
> /proc/vmstat.
>
> Does that sound acceptable?

Yes, I really don't know what's the right thing to do here or where the
best tradeoff point is situated. Let's start off this way and see what
happens I guess.

> --- a/fs/drop_caches.c
> +++ b/fs/drop_caches.c
> @@ -59,10 +59,22 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> if (ret)
> return ret;
> if (write) {
> - if (sysctl_drop_caches & 1)
> + static int stfu;

That identifier wasn't serious, but I kinda like it.

> +
> + if (sysctl_drop_caches & 1) {
> iterate_supers(drop_pagecache_sb, NULL);
> - if (sysctl_drop_caches & 2)
> + count_vm_event(DROP_PAGECACHE);
> + }
> + if (sysctl_drop_caches & 2) {
> drop_slab();
> + count_vm_event(DROP_SLAB);
> + }
> + if (!stfu) {
> + pr_info("%s (%d): drop_caches: %d\n",
> + current->comm, task_pid_nr(current),
> + sysctl_drop_caches);
> + }
> + stfu |= sysctl_drop_caches & 4;
> }
> return 0;
> }