linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Hugh Dickins <hughd@google.com>,
	Matthew Wilcox <willy@infradead.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Will Deacon <will@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Jan Kara <jack@suse.cz>, Minchan Kim <minchan@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Vinayak Menon <vinmenon@codeaurora.org>,
	Android Kernel Team <kernel-team@android.com>
Subject: Re: [PATCH 1/2] mm: Allow architectures to request 'old' entries when prefaulting
Date: Sat, 26 Dec 2020 09:57:13 -0800	[thread overview]
Message-ID: <CAHk-=wiT50aEErZgZOrbFQ=GhFuM3MnBmHoSBSScN9rmXMnOKQ@mail.gmail.com> (raw)
In-Reply-To: <20201225113157.e7hmluffh56fszfc@box>

On Fri, Dec 25, 2020 at 3:31 AM Kirill A. Shutemov <kirill@shutemov.name> wrote:
>
> The new helper next_page() returns a stablized page, so filemap_map_pmd()
> can clearly decide if we should set up a page table or a huge page.

I really like that next_page() abstraction, my only comment is that I
think it should be renamed as "next_stable_page()" or something, and
then this part:

+       if (!page)
+               page = xas_find(xas, end_pgoff);
+       else
+               page = xas_next_entry(xas, end_pgoff);

should be in the caller.

Then just have two helper functions like 'first_map_page()' and
'next_map_page()' which just do

        next_stable_page(xas_find(xas, end_pgoff))

and

        next_stable_page(xas_next_entry(xas, end_pgoff))

respectively.

Because not only does that get rid of the "if (page)" test, I think it
would make things a bit clearer. When I read the patch first, the
initial "next_page()" call confused me.

But maybe I'm just grasping at straws. Even in this format, I think
it's a nice cleanup and makes for more understandable code.

          Linus

  reply	other threads:[~2020-12-26 17:58 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-09 16:39 [PATCH 0/2] Create 'old' ptes for faultaround mappings on arm64 with hardware access flag Will Deacon
2020-12-09 16:39 ` [PATCH 1/2] mm: Allow architectures to request 'old' entries when prefaulting Will Deacon
2020-12-09 17:58   ` Linus Torvalds
2020-12-09 18:40     ` Will Deacon
2020-12-09 19:04       ` Linus Torvalds
2020-12-09 20:32         ` Matthew Wilcox
2020-12-09 21:04           ` Linus Torvalds
2020-12-10 15:08         ` Kirill A. Shutemov
2020-12-10 17:23           ` Linus Torvalds
2020-12-14 16:07             ` Kirill A. Shutemov
2020-12-14 17:54               ` Linus Torvalds
2020-12-14 18:56                 ` Matthew Wilcox
2020-12-16 17:07                 ` Kirill A. Shutemov
2020-12-16 18:41                   ` Linus Torvalds
2020-12-17 10:54                     ` Kirill A. Shutemov
2020-12-17 18:22                       ` Linus Torvalds
2020-12-18 11:04                         ` Kirill A. Shutemov
2020-12-18 18:56                           ` Linus Torvalds
2020-12-19 12:41                             ` Kirill A. Shutemov
2020-12-19 20:08                               ` Linus Torvalds
2020-12-19 20:34                               ` Linus Torvalds
2020-12-22 10:00                                 ` Kirill A. Shutemov
2020-12-24  4:04                                   ` Hugh Dickins
2020-12-25 11:31                                     ` Kirill A. Shutemov
2020-12-26 17:57                                       ` Linus Torvalds [this message]
2020-12-26 20:43                                         ` Kirill A. Shutemov
2020-12-26 21:03                                           ` Hugh Dickins
2020-12-26 21:16                                             ` Linus Torvalds
2020-12-26 22:40                                               ` Kirill A. Shutemov
2020-12-27  0:45                                                 ` Hugh Dickins
2020-12-27  2:38                                                   ` Hugh Dickins
2020-12-27 19:38                                                     ` Linus Torvalds
2020-12-27 20:32                                                       ` Damian Tometzki
2020-12-27 22:35                                                         ` Hugh Dickins
2020-12-27 23:12                                                           ` Linus Torvalds
2020-12-27 23:40                                                             ` Linus Torvalds
2020-12-27 23:55                                                               ` Kirill A. Shutemov
2020-12-27 23:48                                                       ` Kirill A. Shutemov
2020-12-28  1:54                                                         ` Linus Torvalds
2020-12-28  6:43                                                           ` Hugh Dickins
2020-12-28 12:53                                                             ` Kirill A. Shutemov
2020-12-28 18:47                                                               ` Linus Torvalds
2020-12-28 21:58                                                                 ` Linus Torvalds
2020-12-29 13:28                                                                   ` Kirill A. Shutemov
2020-12-29 15:19                                                                     ` Matthew Wilcox
2020-12-29 20:52                                                                     ` Linus Torvalds
2020-12-28 22:05                                                                 ` Kirill A. Shutemov
2020-12-28 22:12                                                                   ` Kirill A. Shutemov
2020-12-29  4:35                                                                     ` Hugh Dickins
2020-12-28 23:28                                                                   ` Linus Torvalds
2020-12-26 21:07                                           ` Linus Torvalds
2020-12-26 21:41                                           ` Matthew Wilcox
2020-12-09 16:39 ` [PATCH 2/2] arm64: mm: Implement arch_wants_old_faultaround_pte() Will Deacon
2020-12-09 18:35   ` Catalin Marinas
2020-12-09 18:46     ` Will Deacon

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='CAHk-=wiT50aEErZgZOrbFQ=GhFuM3MnBmHoSBSScN9rmXMnOKQ@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --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=kirill@shutemov.name \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=vinmenon@codeaurora.org \
    --cc=will@kernel.org \
    --cc=willy@infradead.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).