From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Will Deacon <will@kernel.org>
Cc: kernel-team@android.com, Jan Kara <jack@suse.cz>,
Minchan Kim <minchan@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Hugh Dickins <hughd@google.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Vinayak Menon <vinmenon@codeaurora.org>,
"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 1/3] mm: Cleanup faultaround and finish_fault() codepaths
Date: Mon, 11 Jan 2021 17:26:20 +0300 [thread overview]
Message-ID: <20210111142620.ylv3ccrsubef62mi@box> (raw)
In-Reply-To: <20210108171517.5290-2-will@kernel.org>
On Fri, Jan 08, 2021 at 05:15:15PM +0000, Will Deacon wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> alloc_set_pte() has two users with different requirements: in the
> faultaround code, it called from an atomic context and PTE page table
> has to be preallocated. finish_fault() can sleep and allocate page table
> as needed.
>
> PTL locking rules are also strange, hard to follow and overkill for
> finish_fault().
>
> Let's untangle the mess. alloc_set_pte() has gone now. All locking is
> explicit.
>
> The price is some code duplication to handle huge pages in faultaround
> path, but it should be fine, having overall improvement in readability.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
> fs/xfs/xfs_file.c | 6 +-
> include/linux/mm.h | 12 ++-
> include/linux/pgtable.h | 11 +++
> mm/filemap.c | 177 ++++++++++++++++++++++++++---------
> mm/memory.c | 199 ++++++++++++----------------------------
> 5 files changed, 213 insertions(+), 192 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 5b0f93f73837..111fe73bb8a7 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1319,17 +1319,19 @@ xfs_filemap_pfn_mkwrite(
> return __xfs_filemap_fault(vmf, PE_SIZE_PTE, true);
> }
>
> -static void
> +static vm_fault_t
> xfs_filemap_map_pages(
> struct vm_fault *vmf,
> pgoff_t start_pgoff,
> pgoff_t end_pgoff)
> {
> struct inode *inode = file_inode(vmf->vma->vm_file);
> + vm_fault_t ret;
>
> xfs_ilock(XFS_I(inode), XFS_MMAPLOCK_SHARED);
> - filemap_map_pages(vmf, start_pgoff, end_pgoff);
> + ret = filemap_map_pages(vmf, start_pgoff, end_pgoff);
> xfs_iunlock(XFS_I(inode), XFS_MMAPLOCK_SHARED);
> + return ret;
> }
>
> static const struct vm_operations_struct xfs_file_vm_ops = {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index ecdf8a8cd6ae..801dd99f733c 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -542,8 +542,8 @@ struct vm_fault {
> * is not NULL, otherwise pmd.
> */
> pgtable_t prealloc_pte; /* Pre-allocated pte page table.
> - * vm_ops->map_pages() calls
> - * alloc_set_pte() from atomic context.
> + * vm_ops->map_pages() sets up a page
> + * table from from atomic context.
> * do_fault_around() pre-allocates
> * page table to avoid allocation from
> * atomic context.
The typo Matthew has pointed out:
diff --git a/include/linux/mm.h b/include/linux/mm.h
index d3d4e307fa09..358fc8616d8b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -535,7 +535,7 @@ struct vm_fault {
*/
pgtable_t prealloc_pte; /* Pre-allocated pte page table.
* vm_ops->map_pages() sets up a page
- * table from from atomic context.
+ * table from atomic context.
* do_fault_around() pre-allocates
* page table to avoid allocation from
* atomic context.
--
Kirill A. Shutemov
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-01-11 14:29 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-08 17:15 [PATCH v2 0/3] Create 'old' ptes for faultaround mappings on arm64 with hardware access flag Will Deacon
2021-01-08 17:15 ` [PATCH v2 1/3] mm: Cleanup faultaround and finish_fault() codepaths Will Deacon
2021-01-11 14:26 ` Kirill A. Shutemov [this message]
2021-01-11 14:27 ` Will Deacon
2021-01-08 17:15 ` [PATCH v2 2/3] mm: Allow architectures to request 'old' entries when prefaulting Will Deacon
2021-01-11 14:25 ` Kirill A. Shutemov
2021-01-11 14:37 ` Will Deacon
2021-01-11 14:47 ` Kirill A. Shutemov
2021-01-08 17:15 ` [PATCH v2 3/3] arm64: mm: Implement arch_wants_old_prefaulted_pte() Will Deacon
2021-01-08 19:34 ` [PATCH v2 0/3] Create 'old' ptes for faultaround mappings on arm64 with hardware access flag Linus Torvalds
2021-01-08 19:42 ` Linus Torvalds
2021-01-11 14:01 ` Will Deacon
2021-01-11 13:30 ` Will Deacon
2021-01-11 21:03 ` Hugh Dickins
2021-01-12 21:46 ` Will Deacon
2021-01-11 14:24 ` Kirill A. Shutemov
2021-01-11 19:25 ` Linus Torvalds
2021-01-12 21:47 ` Will Deacon
2021-01-12 21:55 ` Linus Torvalds
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210111142620.ylv3ccrsubef62mi@box \
--to=kirill@shutemov.name \
--cc=akpm@linux-foundation.org \
--cc=catalin.marinas@arm.com \
--cc=hughd@google.com \
--cc=jack@suse.cz \
--cc=kernel-team@android.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=vinmenon@codeaurora.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).