Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753288AbdIRKWs (ORCPT ); Mon, 18 Sep 2017 06:22:48 -0400 Received: from mx2.suse.de ([195.135.220.15]:39596 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752374AbdIRKWr (ORCPT ); Mon, 18 Sep 2017 06:22:47 -0400 Date: Mon, 18 Sep 2017 12:22:44 +0200 From: Jan Kara To: Yafang Shao Cc: akpm@linux-foundation.org, jack@suse.cz, hannes@cmpxchg.org, mhocko@suse.com, vdavydov.dev@gmail.com, jlayton@redhat.com, nborisov@suse.com, tytso@mit.edu, mawilcox@microsoft.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: introduce sanity check on dirty ratio sysctl value Message-ID: <20170918102244.GJ32516@quack2.suse.cz> References: <1505669968-12593-1-git-send-email-laoar.shao@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1505669968-12593-1-git-send-email-laoar.shao@gmail.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1632 Lines: 59 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 ... > { > + 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 SUSE Labs, CR