Re: [PATCH] mm, thp: Relax the VM_DENYWRITE constraint on file-backed THPs

From: Collin Fijalkovich <cfijalkovich@google.com>
To: Song Liu <songliubraving@fb.com>
Cc: Song Liu <song@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Hridya Valsaraju <hridya@google.com>,
	Kalesh Singh <kaleshsingh@google.com>,
	Hugh Dickins <hughd@google.com>,
	Tim Murray <timmurray@google.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux-Fsdevel <linux-fsdevel@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>
Subject: Re: [PATCH] mm, thp: Relax the VM_DENYWRITE constraint on file-backed THPs
Date: Tue, 30 Mar 2021 13:00:59 -0700	[thread overview]
Message-ID: <CAL+PeoHXNjcgR=te+WnkGGMiGyqqdparX+HH8K2KCK0CV9sUKg@mail.gmail.com> (raw)
In-Reply-To: <2E59E29C-E04D-417C-9B2B-7F0F7D5E43EA@fb.com>

There will be an immediate performance hit when the file is opened for
write, as its associated pages are removed from the page cache. While
the writer is present there will be the usual overhead of using 4kB
pages instead of THPs, but there should not be an additional penalty.
It is problematic if a file is repeatedly opened for write, as it will
need to refault each time.

- Collin

On Sun, Mar 28, 2021 at 9:45 AM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Mar 23, 2021, at 10:13 AM, Collin Fijalkovich <cfijalkovich@google.com> wrote:
> >
> > Question: when we use this on shared library, the library is still
> > writable. When the
> > shared library is opened for write, these pages will refault in as 4kB
> > pages, right?
> >
> > That's correct, while a file is opened for write it will refault into 4kB pages and block use of THPs. Once all writers complete (i_writecount <=0), the file can fault into THPs again and khugepaged can collapse existing page ranges provided that it can successfully allocate new huge pages.
>
> Will it be a problem if a slow writer (say a slow scp) writes to the
> shared library while the shared library is in use?
>
> Thanks,
> Song
>
> >
> > From,
> > Collin
> >
> > On Mon, Mar 22, 2021 at 4:55 PM Song Liu <song@kernel.org> wrote:
> > On Mon, Mar 22, 2021 at 3:00 PM Collin Fijalkovich
> > <cfijalkovich@google.com> wrote:
> > >
> > > Transparent huge pages are supported for read-only non-shmem filesystems,
> > > but are only used for vmas with VM_DENYWRITE. This condition ensures that
> > > file THPs are protected from writes while an application is running
> > > (ETXTBSY).  Any existing file THPs are then dropped from the page cache
> > > when a file is opened for write in do_dentry_open(). Since sys_mmap
> > > ignores MAP_DENYWRITE, this constrains the use of file THPs to vmas
> > > produced by execve().
> > >
> > > Systems that make heavy use of shared libraries (e.g. Android) are unable
> > > to apply VM_DENYWRITE through the dynamic linker, preventing them from
> > > benefiting from the resultant reduced contention on the TLB.
> > >
> > > This patch reduces the constraint on file THPs allowing use with any
> > > executable mapping from a file not opened for write (see
> > > inode_is_open_for_write()). It also introduces additional conditions to
> > > ensure that files opened for write will never be backed by file THPs.
> >
> > Thanks for working on this. We could also use this in many data center
> > workloads.
> >
> > Question: when we use this on shared library, the library is still
> > writable. When the
> > shared library is opened for write, these pages will refault in as 4kB
> > pages, right?
> >
> > Thanks,
> > Song
>