Received-SPF: pass (google.com: domain of linux-kernel+bounces-27471-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <1bc8a5df-b413-4869-8931-98f5b9e82fe5@suse.cz>
Date: Tue, 16 Jan 2024 15:42:01 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC 0/3] reading proc/pid/maps under RCU
Content-Language: en-US
To: Suren Baghdasaryan <surenb@google.com>, akpm@linux-foundation.org
Cc: viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz,
 dchinner@redhat.com, casey@schaufler-ca.com, ben.wolsieffer@hefring.com,
 paulmck@kernel.org, david@redhat.com, avagin@google.com,
 usama.anjum@collabora.com, peterx@redhat.com, hughd@google.com,
 ryan.roberts@arm.com, wangkefeng.wang@huawei.com, Liam.Howlett@Oracle.com,
 yuzhao@google.com, axelrasmussen@google.com, lstoakes@gmail.com,
 talumbau@google.com, willy@infradead.org, mgorman@techsingularity.net,
 jhubbard@nvidia.com, vishal.moola@gmail.com, mathieu.desnoyers@efficios.com,
 dhowells@redhat.com, jgg@ziepe.ca, sidhartha.kumar@oracle.com,
 andriy.shevchenko@linux.intel.com, yangxingui@huawei.com,
 keescook@chromium.org, linux-kernel@vger.kernel.org,
 linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, kernel-team@android.com
References: <20240115183837.205694-1-surenb@google.com>
From: Vlastimil Babka <vbabka@suse.cz>
In-Reply-To: <20240115183837.205694-1-surenb@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 1/15/24 19:38, Suren Baghdasaryan wrote:

Hi,

> The issue this patchset is trying to address is mmap_lock contention when
> a low priority task (monitoring, data collecting, etc.) blocks a higher
> priority task from making updated to the address space. The contention is
> due to the mmap_lock being held for read when reading proc/pid/maps.
> With maple_tree introduction, VMA tree traversals are RCU-safe and per-vma
> locks make VMA access RCU-safe. this provides an opportunity for lock-less
> reading of proc/pid/maps. We still need to overcome a couple obstacles:
> 1. Make all VMA pointer fields used for proc/pid/maps content generation
> RCU-safe;
> 2. Ensure that proc/pid/maps data tearing, which is currently possible at
> page boundaries only, does not get worse.

Hm I thought we were to only choose this more complicated in case additional
tearing becomes a problem, and at first assume that if software can deal
with page boundary tearing, it can deal with sub-page tearing too?

> The patchset deals with these issues but there is a downside which I would
> like to get input on:
> This change introduces unfairness towards the reader of proc/pid/maps,
> which can be blocked by an overly active/malicious address space modifyer.

So this is a consequence of the validate() operation, right? We could avoid
this if we allowed sub-page tearing.

> A couple of ways I though we can address this issue are:
> 1. After several lock-less retries (or some time limit) to fall back to
> taking mmap_lock.
> 2. Employ lock-less reading only if the reader has low priority,
> indicating that blocking it is not critical.
> 3. Introducing a separate procfs file which publishes the same data in
> lock-less manner.
> 
> I imagine a combination of these approaches can also be employed.
> I would like to get feedback on this from the Linux community.
> 
> Note: mmap_read_lock/mmap_read_unlock sequence inside validate_map()
> can be replaced with more efficiend rwsem_wait() proposed by Matthew
> in [1].
> 
> [1] https://lore.kernel.org/all/ZZ1+ZicgN8dZ3zj3@casper.infradead.org/
> 
> Suren Baghdasaryan (3):
>   mm: make vm_area_struct anon_name field RCU-safe
>   seq_file: add validate() operation to seq_operations
>   mm/maps: read proc/pid/maps under RCU
> 
>  fs/proc/internal.h        |   3 +
>  fs/proc/task_mmu.c        | 130 ++++++++++++++++++++++++++++++++++----
>  fs/seq_file.c             |  24 ++++++-
>  include/linux/mm_inline.h |  10 ++-
>  include/linux/mm_types.h  |   3 +-
>  include/linux/seq_file.h  |   1 +
>  mm/madvise.c              |  30 +++++++--
>  7 files changed, 181 insertions(+), 20 deletions(-)
>