linux-kernel - Re: [PATCH v3] mm: introduce validity check on vm dirtiness settings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 26 Sep 2017 13:26:56 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Yafang Shao <laoar.shao@...il.com>
Cc:     Jan Kara <jack@...e.cz>, akpm@...ux-foundation.org,
        Johannes Weiner <hannes@...xchg.org>, vdavydov.dev@...il.com,
        jlayton@...hat.com, nborisov@...e.com,
        Theodore Ts'o <tytso@....edu>, mawilcox@...rosoft.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] mm: introduce validity check on vm dirtiness settings

On Tue 26-09-17 19:06:37, Yafang Shao wrote:
> 2017-09-26 18:25 GMT+08:00 Michal Hocko <mhocko@...nel.org>:
> > On Wed 20-09-17 06:43:35, 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 vm background dirtiness bigger than
> >> vm dirtiness successfully. This behavior may mislead us.
> >> We'd better do this validity check at the beginning.
> >
> > This is an admin only interface. You can screw setting this up even
> > when you keep consistency between the background and direct limits. In
> > general we do not try to be clever for these knobs because we _expect_
> > admins to do sane things. Why is this any different and why do we need
> > to add quite some code to handle one particular corner case?
> >
> 
> Of course we expect admins to do the sane things, but not all admins
> are expert or faimilar with linux kernel source code.
> If we have to read the source code to know what is the right thing to
> do, I don't think this is a good interface, even for the admin.

Well, it is kind of natural to setup background below the direct limit
in general so I am not sure what is so surprising here. Moreover setting
a non default drity limits already requires some expertise. It is not
like an arbitrary value will work just fine...

> Anyway, there's no document on that direct limits should not less than
> background limits.

Then improve the documentation.

> > To be honest I am not entirely sure this is worth the code and the
> > future maintenance burden.
> I'm not sure if this code is a burden for the future maintenance, but
> I think that if we don't introduce this code it is a burden to the
> admins.

anytime we might need to tweak background vs direct limit we would have
to change these checks as well and that sounds like a maint. burden to
me.
-- 
Michal Hocko
SUSE Labs