Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCHSET v3] writeback, memcg: Implement foreign inode flushing
To:     Tejun Heo <tj@kernel.org>, jack@suse.cz, hannes@cmpxchg.org,
        mhocko@kernel.org, vdavydov.dev@gmail.com
Cc:     cgroups@vger.kernel.org, linux-mm@kvack.org,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        kernel-team@fb.com, guro@fb.com, akpm@linux-foundation.org
References: <20190826160656.870307-1-tj@kernel.org>
From:   Jens Axboe <axboe@kernel.dk>
Message-ID: <15a5a6e8-90bf-726b-f68c-db91f1afc651@kernel.dk>
Date:   Tue, 27 Aug 2019 09:23:09 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20190826160656.870307-1-tj@kernel.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 8/26/19 10:06 AM, Tejun Heo wrote:
> Hello,
> 
> Changes from v1[1]:
> 
> * More comments explaining the parameters.
> 
> * 0003-writeback-Separate-out-wb_get_lookup-from-wb_get_create.patch
>    added and avoid spuriously creating missing wbs for foreign
>    flushing.
> 
> Changes from v2[2]:
> 
> * Added livelock avoidance and applied other smaller changes suggested
>    by Jan.
> 
> There's an inherent mismatch between memcg and writeback.  The former
> trackes ownership per-page while the latter per-inode.  This was a
> deliberate design decision because honoring per-page ownership in the
> writeback path is complicated, may lead to higher CPU and IO overheads
> and deemed unnecessary given that write-sharing an inode across
> different cgroups isn't a common use-case.
> 
> Combined with inode majority-writer ownership switching, this works
> well enough in most cases but there are some pathological cases.  For
> example, let's say there are two cgroups A and B which keep writing to
> different but confined parts of the same inode.  B owns the inode and
> A's memory is limited far below B's.  A's dirty ratio can rise enough
> to trigger balance_dirty_pages() sleeps but B's can be low enough to
> avoid triggering background writeback.  A will be slowed down without
> a way to make writeback of the dirty pages happen.
> 
> This patchset implements foreign dirty recording and foreign mechanism
> so that when a memcg encounters a condition as above it can trigger
> flushes on bdi_writebacks which can clean its pages.  Please see the
> last patch for more details.
> 
> This patchset contains the following four patches.
> 
>   0001-writeback-Generalize-and-expose-wb_completion.patch
>   0002-bdi-Add-bdi-id.patch
>   0003-writeback-Separate-out-wb_get_lookup-from-wb_get_create.patch
>   0004-writeback-memcg-Implement-cgroup_writeback_by_id.patch
>   0005-writeback-memcg-Implement-foreign-dirty-flushing.patch
> 
> 0001-0004 are prep patches which expose wb_completion and implement
> bdi->id and flushing by bdi and memcg IDs.
> 
> 0005 implements foreign inode flushing.
> 
> Thanks.  diffstat follows.
> 
>   fs/fs-writeback.c                |  130 ++++++++++++++++++++++++++++---------
>   include/linux/backing-dev-defs.h |   23 ++++++
>   include/linux/backing-dev.h      |    5 +
>   include/linux/memcontrol.h       |   39 +++++++++++
>   include/linux/writeback.h        |    2
>   mm/backing-dev.c                 |  120 +++++++++++++++++++++++++++++-----
>   mm/memcontrol.c                  |  134 +++++++++++++++++++++++++++++++++++++++
>   mm/page-writeback.c              |    4 +
>   8 files changed, 404 insertions(+), 53 deletions(-)

Applied for 5.4, thanks Tejun.

-- 
Jens Axboe