[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 4 May 2021 21:32:04 -0400
From: Peter Xu <peterx@...hat.com>
To: Axel Rasmussen <axelrasmussen@...gle.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
Andrea Arcangeli <aarcange@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Hugh Dickins <hughd@...gle.com>,
Jerome Glisse <jglisse@...hat.com>,
Joe Perches <joe@...ches.com>,
Lokesh Gidra <lokeshgidra@...gle.com>,
Mike Kravetz <mike.kravetz@...cle.com>,
Mike Rapoport <rppt@...ux.vnet.ibm.com>,
Shaohua Li <shli@...com>, Shuah Khan <shuah@...nel.org>,
Stephen Rothwell <sfr@...b.auug.org.au>,
Wang Qing <wangqing@...o.com>, linux-api@...r.kernel.org,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-mm@...ck.org,
Brian Geffon <bgeffon@...gle.com>,
"Dr . David Alan Gilbert" <dgilbert@...hat.com>,
Mina Almasry <almasrymina@...gle.com>,
Oliver Upton <oupton@...gle.com>
Subject: Re: [PATCH v6 06/10] userfaultfd/shmem: modify
shmem_mfill_atomic_pte to use install_pte()
Axel,
On Mon, May 03, 2021 at 11:07:33AM -0700, Axel Rasmussen wrote:
> In a previous commit, we added the mfill_atomic_install_pte() helper.
> This helper does the job of setting up PTEs for an existing page, to map
> it into a given VMA. It deals with both the anon and shmem cases, as
> well as the shared and private cases.
>
> In other words, shmem_mfill_atomic_pte() duplicates a case it already
> handles. So, expose it, and let shmem_mfill_atomic_pte() use it
> directly, to reduce code duplication.
>
> This requires that we refactor shmem_mfill_atomic_pte() a bit:
>
> Instead of doing accounting (shmem_recalc_inode() et al) part-way
> through the PTE setup, do it afterward. This frees up
> mfill_atomic_install_pte() from having to care about this accounting,
> and means we don't need to e.g. shmem_uncharge() in the error path.
>
> A side effect is this switches shmem_mfill_atomic_pte() to use
> lru_cache_add_inactive_or_unevictable() instead of just lru_cache_add().
> This wrapper does some extra accounting in an exceptional case, if
> appropriate, so it's actually the more correct thing to use.
>
> Signed-off-by: Axel Rasmussen <axelrasmussen@...gle.com>
(The moving of "ret = -ENOMEM" seems unnecessary, but not a big deal I think)
Reviewed-by: Peter Xu <peterx@...hat.com>
Thanks,
--
Peter Xu
Powered by blists - more mailing lists