| Message ID | 157337934693.2078.9842146413181153727.stgit@buzz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] fs/quota: handle overflows of sysctl fs.quota.* and report as unsigned long | expand |
On Sun 10-11-19 12:49:06, Konstantin Khlebnikov wrote: > Quota statistics counted as 64-bit per-cpu counter. Reading sums per-cpu > fractions as signed 64-bit int, filters negative values and then reports > lower half as signed 32-bit int. > > Result may looks like: > > fs.quota.allocated_dquots = 22327 > fs.quota.cache_hits = -489852115 > fs.quota.drops = -487288718 > fs.quota.free_dquots = 22083 > fs.quota.lookups = -486883485 > fs.quota.reads = 22327 > fs.quota.syncs = 335064 > fs.quota.writes = 3088689 > > Values bigger than 2^31-1 reported as negative. > > All counters except "allocated_dquots" and "free_dquots" are monotonic, > thus they should be reported as is without filtering negative values. > > Kernel doesn't have generic helper for 64-bit sysctl yet, > let's use at least unsigned long. > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Thanks! I've added the patch to my tree. Honza > --- > fs/quota/dquot.c | 29 +++++++++++++++++------------ > include/linux/quota.h | 2 +- > 2 files changed, 18 insertions(+), 13 deletions(-) > > diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c > index 6e826b454082..fa6ec4f96791 100644 > --- a/fs/quota/dquot.c > +++ b/fs/quota/dquot.c > @@ -2860,68 +2860,73 @@ EXPORT_SYMBOL(dquot_quotactl_sysfile_ops); > static int do_proc_dqstats(struct ctl_table *table, int write, > void __user *buffer, size_t *lenp, loff_t *ppos) > { > - unsigned int type = (int *)table->data - dqstats.stat; > + unsigned int type = (unsigned long *)table->data - dqstats.stat; > + s64 value = percpu_counter_sum(&dqstats.counter[type]); > + > + /* Filter negative values for non-monotonic counters */ > + if (value < 0 && (type == DQST_ALLOC_DQUOTS || > + type == DQST_FREE_DQUOTS)) > + value = 0; > > /* Update global table */ > - dqstats.stat[type] = > - percpu_counter_sum_positive(&dqstats.counter[type]); > - return proc_dointvec(table, write, buffer, lenp, ppos); > + dqstats.stat[type] = value; > + return proc_doulongvec_minmax(table, write, buffer, lenp, ppos); > } > > static struct ctl_table fs_dqstats_table[] = { > { > .procname = "lookups", > .data = &dqstats.stat[DQST_LOOKUPS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(unsigned long), > .mode = 0444, > .proc_handler = do_proc_dqstats, > }, > { > .procname = "drops", > .data = &dqstats.stat[DQST_DROPS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(unsigned long), > .mode = 0444, > .proc_handler = do_proc_dqstats, > }, > { > .procname = "reads", > .data = &dqstats.stat[DQST_READS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(unsigned long), > .mode = 0444, > .proc_handler = do_proc_dqstats, > }, > { > .procname = "writes", > .data = &dqstats.stat[DQST_WRITES], > - .maxlen = sizeof(int), > + .maxlen = sizeof(unsigned long), > .mode = 0444, > .proc_handler = do_proc_dqstats, > }, > { > .procname = "cache_hits", > .data = &dqstats.stat[DQST_CACHE_HITS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(unsigned long), > .mode = 0444, > .proc_handler = do_proc_dqstats, > }, > { > .procname = "allocated_dquots", > .data = &dqstats.stat[DQST_ALLOC_DQUOTS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(unsigned long), > .mode = 0444, > .proc_handler = do_proc_dqstats, > }, > { > .procname = "free_dquots", > .data = &dqstats.stat[DQST_FREE_DQUOTS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(unsigned long), > .mode = 0444, > .proc_handler = do_proc_dqstats, > }, > { > .procname = "syncs", > .data = &dqstats.stat[DQST_SYNCS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(unsigned long), > .mode = 0444, > .proc_handler = do_proc_dqstats, > }, > diff --git a/include/linux/quota.h b/include/linux/quota.h > index f32dd270b8e3..27aab84fcbaa 100644 > --- a/include/linux/quota.h > +++ b/include/linux/quota.h > @@ -263,7 +263,7 @@ enum { > }; > > struct dqstats { > - int stat[_DQST_DQSTAT_LAST]; > + unsigned long stat[_DQST_DQSTAT_LAST]; > struct percpu_counter counter[_DQST_DQSTAT_LAST]; > }; > >
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 6e826b454082..fa6ec4f96791 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2860,68 +2860,73 @@ EXPORT_SYMBOL(dquot_quotactl_sysfile_ops); static int do_proc_dqstats(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos) { - unsigned int type = (int *)table->data - dqstats.stat; + unsigned int type = (unsigned long *)table->data - dqstats.stat; + s64 value = percpu_counter_sum(&dqstats.counter[type]); + + /* Filter negative values for non-monotonic counters */ + if (value < 0 && (type == DQST_ALLOC_DQUOTS || + type == DQST_FREE_DQUOTS)) + value = 0; /* Update global table */ - dqstats.stat[type] = - percpu_counter_sum_positive(&dqstats.counter[type]); - return proc_dointvec(table, write, buffer, lenp, ppos); + dqstats.stat[type] = value; + return proc_doulongvec_minmax(table, write, buffer, lenp, ppos); } static struct ctl_table fs_dqstats_table[] = { { .procname = "lookups", .data = &dqstats.stat[DQST_LOOKUPS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "drops", .data = &dqstats.stat[DQST_DROPS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "reads", .data = &dqstats.stat[DQST_READS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "writes", .data = &dqstats.stat[DQST_WRITES], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "cache_hits", .data = &dqstats.stat[DQST_CACHE_HITS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "allocated_dquots", .data = &dqstats.stat[DQST_ALLOC_DQUOTS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "free_dquots", .data = &dqstats.stat[DQST_FREE_DQUOTS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "syncs", .data = &dqstats.stat[DQST_SYNCS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, diff --git a/include/linux/quota.h b/include/linux/quota.h index f32dd270b8e3..27aab84fcbaa 100644 --- a/include/linux/quota.h +++ b/include/linux/quota.h @@ -263,7 +263,7 @@ enum { }; struct dqstats { - int stat[_DQST_DQSTAT_LAST]; + unsigned long stat[_DQST_DQSTAT_LAST]; struct percpu_counter counter[_DQST_DQSTAT_LAST]; };
Quota statistics counted as 64-bit per-cpu counter. Reading sums per-cpu fractions as signed 64-bit int, filters negative values and then reports lower half as signed 32-bit int. Result may looks like: fs.quota.allocated_dquots = 22327 fs.quota.cache_hits = -489852115 fs.quota.drops = -487288718 fs.quota.free_dquots = 22083 fs.quota.lookups = -486883485 fs.quota.reads = 22327 fs.quota.syncs = 335064 fs.quota.writes = 3088689 Values bigger than 2^31-1 reported as negative. All counters except "allocated_dquots" and "free_dquots" are monotonic, thus they should be reported as is without filtering negative values. Kernel doesn't have generic helper for 64-bit sysctl yet, let's use at least unsigned long. Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> --- fs/quota/dquot.c | 29 +++++++++++++++++------------ include/linux/quota.h | 2 +- 2 files changed, 18 insertions(+), 13 deletions(-)