/proc/sys/vm/drop_caches: add error handling
Subsystems (3)
Document that drop_caches is unsafe, and add error checking so that it bails out on invalid inputs. [Note that this was triggered by Android trying to use it in production, and incidentally writing invalid value...]
Documentation/sysctl/vm.txt +4 -0
@@ -132,6 +132,10 @@ To free pagecache, dentries and inodes: As this is a non-destructive operation and dirty objects are not freeable, the user should run `sync' first. +Note that calling this causes some serious latencies, and that this is +a debug feature; it should not be used for production as it does not +contain neccessary locking to guarantee safe operation. + ============================================================== hugepages_treat_as_movable
fs/drop_caches.c +16 -10
@@ -9,8 +9,9 @@#include <linux/sysctl.h> #include <linux/gfp.h> -/* A global variable is a bit ugly, but it keeps the code simple */ -int sysctl_drop_caches; +/* A global variable is a bit ugly, and has locking problems, +but it keeps the code simple */ +unsigned int sysctl_drop_caches;static void drop_pagecache_sb (struct super_block *sb) {
@@ -65,12 +66,17 @@ static void drop_slab(void)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 (); - } -return 0 ; +int res; + res =proc_dointvec_minmax (table, write, file, buffer, length, ppos); +if (res) +return res; +if (!write) +return res; +if (sysctl_drop_caches & ~3 ) +return -EINVAL ; +if (sysctl_drop_caches &1 ) +drop_pagecache (); +if (sysctl_drop_caches &2 ) +drop_slab (); +return res; }
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 5 lines
Document that
drop_cachesis unsafe, and add error checking so that it bails out on invalid inputs. [Note that this was triggered by Android trying to use it in production, and incidentally writing invalid value...]
Yup. good patch.
Quoted 5 lines
fs/drop_caches.c +4
-1-return 0 ; +int res; + res =proc_dointvec_minmax (table, write, file, buffer, length, ppos); +if (res) +return res;
fs/drop_caches.c static void drop_slab(void) · line 82
Quoted 10 lines
fs/drop_caches.c +9
-0+if (!write) +return res; +if (sysctl_drop_caches & ~3 ) +return -EINVAL ; +if (sysctl_drop_caches &1 ) +drop_pagecache (); +if (sysctl_drop_caches &2 ) +drop_slab (); +return res; }
I think following is clarify more.
res = proc_dointvec_minmax(table, write, file, buffer, length, ppos); if (res) return res; if (!write) return 0; if (sysctl_drop_caches & ~3) return -EINVAL; if (sysctl_drop_caches & 1) drop_pagecache(); if (sysctl_drop_caches & 2) drop_slab(); return 0;
otherthings, _very_ looks good to me. :)
--
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>
Andrew Morton
maintainerQuoted 40 lines · On Tue, 3 Feb 2009 20:47:56 +0900 (JST) KOSAKI Motohiro…
Quoted 5 lines
Document that
drop_cachesis unsafe, and add error checking so that it bails out on invalid inputs. [Note that this was triggered by Android trying to use it in production, and incidentally writing invalid value...]Yup. good patch.
Quoted 5 lines
fs/drop_caches.c +4
-1-return 0 ; +int res; + res =proc_dointvec_minmax (table, write, file, buffer, length, ppos); +if (res) +return res;fs/drop_caches.c static void drop_slab(void) · line 82
Quoted 10 lines
fs/drop_caches.c +9
-0+if (!write) +return res; +if (sysctl_drop_caches & ~3 ) +return -EINVAL ; +if (sysctl_drop_caches &1 ) +drop_pagecache (); +if (sysctl_drop_caches &2 ) +drop_slab (); +return res; }I think following is clarify more.
res = proc_dointvec_minmax(table, write, file, buffer, length, ppos); if (res) return res; if (!write) return 0; if (sysctl_drop_caches & ~3) return -EINVAL; if (sysctl_drop_caches & 1) drop_pagecache(); if (sysctl_drop_caches & 2) drop_slab(); return 0;
Quoted 2 lines
otherthings, _very_ looks good to me. :)
For better or for worse, my intent here was to be future-back-compatible. So if we later add new flags, and people write code which uses those new flags, that code won't break on old kernels.
Probably that wasn't a very good idea, and such userspace code isn't very good.
--
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>
Hi!
Quoted 16 lines
Quoted 16 lines
I think following is clarify more.
res = proc_dointvec_minmax(table, write, file, buffer, length, ppos); if (res) return res; if (!write) return 0; if (sysctl_drop_caches & ~3) return -EINVAL; if (sysctl_drop_caches & 1) drop_pagecache(); if (sysctl_drop_caches & 2) drop_slab(); return 0;
Quoted 5 lines
Quoted 1 line
otherthings, _very_ looks good to me. :)
For better or for worse, my intent here was to be future-back-compatible. So if we later add new flags, and people write code which uses those new flags, that code won't break on old kernels.
Well, should I move test for &~3 to the end? You'd still get compatibility, but you'd also get expected error reports.
Quoted 2 lines
Probably that wasn't a very good idea, and such userspace code isn't very good.
---
Report errors in drop_caches, and document stuff a bit better.
Documentation/sysctl/vm.txt +4 -0
@@ -132,6 +132,10 @@ To free pagecache, dentries and inodes: As this is a non-destructive operation and dirty objects are not freeable, the user should run `sync' first. +Note that calling this causes some serious latencies, and that this is +a debug feature; it should not be used for production as it does not +contain neccessary locking to guarantee safe operation. + ============================================================== hugepages_treat_as_movable
fs/drop_caches.c +15 -9
@@ -9,8 +9,9 @@#include <linux/sysctl.h> #include <linux/gfp.h> -/* A global variable is a bit ugly, but it keeps the code simple */ -int sysctl_drop_caches; +/* A global variable is a bit ugly, and has locking problems, +but it keeps the code simple */ +unsigned int sysctl_drop_caches;static void drop_pagecache_sb (struct super_block *sb) {
@@ -65,12 +66,17 @@ static void drop_slab(void)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 (); - } +int res; + res =proc_dointvec_minmax (table, write, file, buffer, length, ppos); +if (res) +return res; +if (!write) +return res; +if (sysctl_drop_caches &1 ) +drop_pagecache (); +if (sysctl_drop_caches &2 ) +drop_slab (); +if (sysctl_drop_caches & ~3 ) +return -EINVAL ;return 0 ; }
Signature · 8 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- 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>