Messages in this thread | | | From | Yafang Shao <> | Date | Mon, 18 Sep 2017 19:36:56 +0800 | Subject | Re: [PATCH] mm: introduce sanity check on dirty ratio sysctl value |
| |
2017-09-18 18:22 GMT+08:00 Jan Kara <jack@suse.cz>: > On Mon 18-09-17 01:39:28, Yafang Shao wrote: >> we can find the logic in domain_dirty_limits() that >> when dirty bg_thresh is bigger than dirty thresh, >> bg_thresh will be set as thresh * 1 / 2. >> if (bg_thresh >= thresh) >> bg_thresh = thresh / 2; >> >> But actually we can set dirty_background_raio bigger than >> dirty_ratio successfully. This behavior may mislead us. >> So we should do this sanity check at the beginning. >> >> Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > ... > >> { >> + int old_ratio = dirty_background_ratio; >> + unsigned long bytes; >> int ret; >> >> ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); >> - if (ret == 0 && write) >> - dirty_background_bytes = 0; >> + >> + if (ret == 0 && write) { >> + if (vm_dirty_ratio > 0) { >> + if (dirty_background_ratio >= vm_dirty_ratio) >> + ret = -EINVAL; >> + } else if (vm_dirty_bytes > 0) { >> + bytes = global_dirtyable_memory() * PAGE_SIZE * >> + dirty_background_ratio / 100; >> + if (bytes >= vm_dirty_bytes) >> + ret = -EINVAL; >> + } >> + >> + if (ret == 0) >> + dirty_background_bytes = 0; >> + else >> + dirty_background_ratio = old_ratio; >> + } >> + > > How about implementing something like > > bool vm_dirty_settings_valid(void) > > helper which would validate whether current dirtiness settings are > consistent. That way we would not have to repeat very similar checks four > times.
That seems a smarter way.
> Also the arithmetics in: > > global_dirtyable_memory() * PAGE_SIZE * dirty_background_ratio / 100 > > could overflow so I'd prefer to first divide by 100 and then multiply by > dirty_background_ratio... > Oh, yes. It could overflow.
> Honza > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR
I will reimplement it and submit a new patch.
Thanks Yafang
| |