[PATCH v2] mm: introduce validity check on vm dirtiness settings

* [PATCH v2] mm: introduce validity check on vm dirtiness settings
@ 2017-09-18 22:53 Yafang Shao
  2017-09-19  8:35 ` Jan Kara
  0 siblings, 1 reply; 5+ messages in thread
From: Yafang Shao @ 2017-09-18 22:53 UTC (permalink / raw)
  To: jack
  Cc: akpm, hannes, mhocko, vdavydov.dev, jlayton, nborisov, tytso,
	mawilcox, linux-mm, linux-kernel, laoar.shao

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 vm background dirtiness bigger than
vm dirtiness successfully. This behavior may mislead us.
We'd better do this validity check at the beginning.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 Documentation/sysctl/vm.txt |  5 +++
 mm/page-writeback.c         | 86 ++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 83 insertions(+), 8 deletions(-)

diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
index 9baf66a..5de02f6 100644
--- a/Documentation/sysctl/vm.txt
+++ b/Documentation/sysctl/vm.txt
@@ -156,6 +156,8 @@ read.
 Note: the minimum value allowed for dirty_bytes is two pages (in bytes); any
 value lower than this limit will be ignored and the old configuration will be
 retained.
+dirty_bytes can't less than dirty_background_bytes or
+available_memory / 100 * dirty_ratio.
 
 ==============================================================
 
@@ -176,6 +178,9 @@ generating disk writes will itself start writing out dirty data.
 
 The total available memory is not equal to total system memory.
 
+Note: dirty_ratio can't less than dirty_background_ratio or
+dirty_background_bytes / available_memory * 100.
+
 ==============================================================
 
 dirty_writeback_centisecs
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0b9c5cb..4a2d229 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -511,15 +511,68 @@ bool node_dirty_ok(struct pglist_data *pgdat)
 	return nr_pages <= limit;
 }
 
+static bool vm_dirty_settings_valid(void)
+{
+	bool ret = true;
+	unsigned long bytes;
+
+	if (vm_dirty_ratio > 0) {
+		if (dirty_background_ratio >= vm_dirty_ratio) {
+			ret = false;
+			goto out;
+		}
+
+		bytes = global_dirtyable_memory() * PAGE_SIZE / 100 *
+				vm_dirty_ratio;
+		if (dirty_background_bytes >= bytes) {
+			ret = false;
+			goto out;
+		}
+	}
+
+	if (vm_dirty_bytes > 0) {
+		if (dirty_background_bytes >= vm_dirty_bytes) {
+			ret = false;
+			goto out;
+		}
+
+		bytes = global_dirtyable_memory() * PAGE_SIZE / 100 *
+				dirty_background_ratio;
+
+		if (bytes >= vm_dirty_bytes) {
+			ret = false;
+			goto out;
+		}
+	}
+
+	if (vm_dirty_bytes == 0 && vm_dirty_ratio == 0 &&
+		(dirty_background_bytes != 0 || dirty_background_ratio != 0))
+		ret = false;
+
+out:
+	if (!ret)
+		pr_err("vm dirtiness can't less than vm background dirtiness\n");
+
+	return ret;
+}
+
 int dirty_background_ratio_handler(struct ctl_table *table, int write,
 		void __user *buffer, size_t *lenp,
 		loff_t *ppos)
 {
 	int ret;
+	int old_ratio = dirty_background_ratio;
 
 	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
-	if (ret == 0 && write)
-		dirty_background_bytes = 0;
+	if (ret == 0 && write) {
+		if (dirty_background_ratio != old_ratio &&
+			!vm_dirty_settings_valid()) {
+			dirty_background_ratio = old_ratio;
+			ret = -EINVAL;
+		} else
+			dirty_background_bytes = 0;
+	}
+
 	return ret;
 }
 
@@ -528,10 +581,17 @@ int dirty_background_bytes_handler(struct ctl_table *table, int write,
 		loff_t *ppos)
 {
 	int ret;
+	unsigned long old_bytes = dirty_background_bytes;
 
 	ret = proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
-	if (ret == 0 && write)
-		dirty_background_ratio = 0;
+	if (ret == 0 && write) {
+		if (dirty_background_bytes != old_bytes &&
+			!vm_dirty_settings_valid()) {
+			dirty_background_bytes = old_bytes;
+			ret = -EINVAL;
+		} else
+			dirty_background_ratio = 0;
+	}
 	return ret;
 }
 
@@ -544,8 +604,13 @@ int dirty_ratio_handler(struct ctl_table *table, int write,
 
 	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
 	if (ret == 0 && write && vm_dirty_ratio != old_ratio) {
-		writeback_set_ratelimit();
-		vm_dirty_bytes = 0;
+		if (vm_dirty_settings_valid()) {
+			writeback_set_ratelimit();
+			vm_dirty_bytes = 0;
+		} else {
+			vm_dirty_ratio = old_ratio;
+			ret = -EINVAL;
+		}
 	}
 	return ret;
 }
@@ -559,8 +624,13 @@ int dirty_bytes_handler(struct ctl_table *table, int write,
 
 	ret = proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
 	if (ret == 0 && write && vm_dirty_bytes != old_bytes) {
-		writeback_set_ratelimit();
-		vm_dirty_ratio = 0;
+		if (vm_dirty_settings_valid()) {
+			writeback_set_ratelimit();
+			vm_dirty_ratio = 0;
+		} else {
+			vm_dirty_bytes = old_bytes;
+			ret = -EINVAL;
+		}
 	}
 	return ret;
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread