2008-11-13 12:22:34

by Nikanth Karthikesan

Subject: [PATCH] Make /proc/sys/vm/drop_caches write only

/proc/sys/vm/drop_caches is used only to trigger dropping caches. It does not
disable the cache. The ability to read the value written to this file long
back gives wrong impression.

Signed-off-by: Nikanth Karthikesan <[email protected]>

---

diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 3e5637f..9892e1b 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -66,11 +66,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
struct file *file, void __user *buffer, size_t *length, loff_t *ppos)
{
proc_dointvec_minmax(table, write, file, buffer, length, ppos);
- if (write) {
- if (sysctl_drop_caches & 1)
- drop_pagecache();
- if (sysctl_drop_caches & 2)
- drop_slab();
- }
+ if (sysctl_drop_caches & 1)
+ drop_pagecache();
+ if (sysctl_drop_caches & 2)
+ drop_slab();
return 0;
}
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 9d048fa..dff3c13 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1015,7 +1015,7 @@ static struct ctl_table vm_table[] = {
.procname = "drop_caches",
.data = &sysctl_drop_caches,
.maxlen = sizeof(int),
- .mode = 0644,
+ .mode = 0200,
.proc_handler = drop_caches_sysctl_handler,
.strategy = &sysctl_intvec,
},

2008-11-14 23:07:30

by Andrew Morton

Subject: Re: [PATCH] Make /proc/sys/vm/drop_caches write only

On Thu, 13 Nov 2008 17:54:11 +0530
Nikanth Karthikesan <[email protected]> wrote:

> /proc/sys/vm/drop_caches is used only to trigger dropping caches. It does not
> disable the cache. The ability to read the value written to this file long
> back gives wrong impression.
>
> Signed-off-by: Nikanth Karthikesan <[email protected]>
>
> ---
>
> diff --git a/fs/drop_caches.c b/fs/drop_caches.c
> index 3e5637f..9892e1b 100644
> --- a/fs/drop_caches.c
> +++ b/fs/drop_caches.c
> @@ -66,11 +66,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> struct file *file, void __user *buffer, size_t *length, loff_t *ppos)
> {
> proc_dointvec_minmax(table, write, file, buffer, length, ppos);
> - if (write) {
> - if (sysctl_drop_caches & 1)
> - drop_pagecache();
> - if (sysctl_drop_caches & 2)
> - drop_slab();
> - }
> + if (sysctl_drop_caches & 1)
> + drop_pagecache();
> + if (sysctl_drop_caches & 2)
> + drop_slab();
> return 0;
> }
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 9d048fa..dff3c13 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1015,7 +1015,7 @@ static struct ctl_table vm_table[] = {
> .procname = "drop_caches",
> .data = &sysctl_drop_caches,
> .maxlen = sizeof(int),
> - .mode = 0644,
> + .mode = 0200,
> .proc_handler = drop_caches_sysctl_handler,
> .strategy = &sysctl_intvec,
> },

True. But it's a backward-incompatible change, and there's a (small)
chance that someone's tools will break if we do this.

We have to get these things right first time :(

2008-11-17 06:26:27

by Nikanth Karthikesan

Subject: Re: [PATCH] Make /proc/sys/vm/drop_caches write only

On Saturday 15 November 2008 04:37:05 Andrew Morton wrote:
> On Thu, 13 Nov 2008 17:54:11 +0530
>
> Nikanth Karthikesan <[email protected]> wrote:
> > /proc/sys/vm/drop_caches is used only to trigger dropping caches. It
> > does not disable the cache. The ability to read the value written to this
> > file long back gives wrong impression.
> >
> > Signed-off-by: Nikanth Karthikesan <[email protected]>
> >
> > ---
> >
> > diff --git a/fs/drop_caches.c b/fs/drop_caches.c
> > index 3e5637f..9892e1b 100644
> > --- a/fs/drop_caches.c
> > +++ b/fs/drop_caches.c
> > @@ -66,11 +66,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int
> > write, struct file *file, void __user *buffer, size_t *length, loff_t
> > *ppos) {
> > proc_dointvec_minmax(table, write, file, buffer, length, ppos);
> > - if (write) {
> > - if (sysctl_drop_caches & 1)
> > - drop_pagecache();
> > - if (sysctl_drop_caches & 2)
> > - drop_slab();
> > - }
> > + if (sysctl_drop_caches & 1)
> > + drop_pagecache();
> > + if (sysctl_drop_caches & 2)
> > + drop_slab();
> > return 0;
> > }
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index 9d048fa..dff3c13 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -1015,7 +1015,7 @@ static struct ctl_table vm_table[] = {
> > .procname = "drop_caches",
> > .data = &sysctl_drop_caches,
> > .maxlen = sizeof(int),
> > - .mode = 0644,
> > + .mode = 0200,
> > .proc_handler = drop_caches_sysctl_handler,
> > .strategy = &sysctl_intvec,
> > },
>
> True. But it's a backward-incompatible change, and there's a (small)
> chance that someone's tools will break if we do this.
>
> We have to get these things right first time :(

hmm.. add this to the list of things for the big version change ;)

May be zeroing out the value would have slimmer chance of breaking things?! Or
would it cause a different confusion?

Thanks
Nikanth Karthikesan

/proc/sys/vm/drop_caches is used only to trigger dropping caches. It
does not disable the cache. Unset it after clearing the cache.

Signed-off-by: Nikanth Karthikesan <[email protected]>

---

diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 3e5637f..1a03d6e 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -71,6 +71,7 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
drop_pagecache();
if (sysctl_drop_caches & 2)
drop_slab();
+ sysctl_drop_caches = 0;
}
return 0;
}