Re: [PATCH] mm: introduce sanity check on dirty ratio sysctl value

From: Jan Kara
Date: Mon Sep 18 2017 - 06:22:52 EST


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@xxxxxxxxx>

...

> {
> + 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. 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...

Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR