linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@linux.ibm.com>
To: Steven Price <steven.price@arm.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>,
	"Mark Rutland" <Mark.Rutland@arm.com>,
	x86@kernel.org, "Arnd Bergmann" <arnd@arndb.de>,
	"Ard Biesheuvel" <ard.biesheuvel@linaro.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"Will Deacon" <will.deacon@arm.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	"Jérôme Glisse" <jglisse@redhat.com>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Borislav Petkov" <bp@alien8.de>,
	"Andy Lutomirski" <luto@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"James Morse" <james.morse@arm.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	linux-arm-kernel@lists.infradead.org, "Liang,
	Kan" <kan.liang@linux.intel.com>
Subject: Re: [PATCH v2 03/13] mm: Add generic p?d_large() macros
Date: Sun, 3 Mar 2019 09:12:53 +0200	[thread overview]
Message-ID: <20190303071253.GA7585@rapoport-lnx> (raw)
In-Reply-To: <b8bd0f99-1c5e-7cf5-32dd-ab52d921e86c@arm.com>

On Fri, Mar 01, 2019 at 01:39:30PM +0000, Steven Price wrote:
> On 01/03/2019 12:30, Kirill A. Shutemov wrote:
> > On Fri, Mar 01, 2019 at 01:53:01PM +0200, Mike Rapoport wrote:
> >> Him Kirill,
> >>
> >> On Fri, Feb 22, 2019 at 12:06:18AM +0300, Kirill A. Shutemov wrote:
> >>> On Thu, Feb 21, 2019 at 05:16:46PM +0000, Steven Price wrote:
> >>>>>> Note that in terms of the new page walking code, these new defines are
> >>>>>> only used when walking a page table without a VMA (which isn't currently
> >>>>>> done), so architectures which don't use p?d_large currently will work
> >>>>>> fine with the generic versions. They only need to provide meaningful
> >>>>>> definitions when switching to use the walk-without-a-VMA functionality.
> >>>>>
> >>>>> How other architectures would know that they need to provide the helpers
> >>>>> to get walk-without-a-VMA functionality? This looks very fragile to me.
> >>>>
> >>>> Yes, you've got a good point there. This would apply to the p?d_large
> >>>> macros as well - any arch which (inadvertently) uses the generic version
> >>>> is likely to be fragile/broken.
> >>>>
> >>>> I think probably the best option here is to scrap the generic versions
> >>>> altogether and simply introduce a ARCH_HAS_PXD_LARGE config option which
> >>>> would enable the new functionality to those arches that opt-in. Do you
> >>>> think this would be less fragile?
> >>>
> >>> These helpers are useful beyond pagewalker.
> >>>
> >>> Can we actually do some grinding and make *all* archs to provide correct
> >>> helpers? Yes, it's tedious, but not that bad.
> >>
> >> Many architectures simply cannot support non-leaf entries at the higher
> >> levels. I think letting the use a generic helper actually does make sense.
> > 
> > I disagree.
> > 
> > It's makes sense if the level doesn't exists on the arch.
> 
> This is what patch 24 [1] of the series does - if the level doesn't
> exist then appropriate stubs are provided.
> 
> > But if the level exists, it will be less frugile to ask the arch to
> > provide the helper. Even if it is dummy always-false.
> 
> The problem (as I see it), is we need a reliable set of p?d_large()
> implementations to be able to walk arbitrary page tables. Either the
> entire functionality of walking page tables without a VMA has to be an
> opt-in per architecture, or we need to mandate that every architecture
> provide these implementations.

I agree that we need a reliable set of p?d_large(), but I'm still not
convinced that every architecture should provide these.

Why having generic versions if p?d_large() is more fragile, than e.g.
p??__access_permitted() or atomic ops?

IMHO, adding those functions/macros for architectures that support large
pages and providing defines to avoid override of 'static inline' implementations
would be robust enough and will avoid unnecessary stubs in architectures
that don't have large pages.
 
> I could provide an asm-generic header to provide a complete set of dummy
> implementations for architectures that don't support large pages at all,
> but that seems a bit overkill when most architectures only need to
> define 2 or 3 implementations (the rest being provided by the
> folded-levels automatically).
> 
> Thanks,
> 
> Steve
> 
> [1]
> https://lore.kernel.org/lkml/20190227170608.27963-25-steven.price@arm.com/

-- 
Sincerely yours,
Mike.


  reply	other threads:[~2019-03-03  7:13 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-21 11:34 [PATCH v2 00/13] Convert x86 & arm64 to use generic page walk Steven Price
2019-02-21 11:34 ` [PATCH v2 01/13] arm64: mm: Add p?d_large() definitions Steven Price
2019-02-21 13:52   ` Mark Rutland
2019-02-21 11:34 ` [PATCH v2 02/13] x86/mm: " Steven Price
2019-02-21 14:21   ` Kirill A. Shutemov
2019-02-21 11:34 ` [PATCH v2 03/13] mm: Add generic p?d_large() macros Steven Price
2019-02-21 13:41   ` Mark Rutland
2019-02-21 14:28   ` Kirill A. Shutemov
2019-02-21 14:46     ` Steven Price
2019-02-21 14:57       ` Kirill A. Shutemov
2019-02-21 17:16         ` Steven Price
2019-02-21 21:06           ` Kirill A. Shutemov
2019-02-22 10:21             ` Steven Price
2019-03-01 11:53             ` Mike Rapoport
2019-03-01 12:30               ` Kirill A. Shutemov
2019-03-01 13:39                 ` Steven Price
2019-03-03  7:12                   ` Mike Rapoport [this message]
2019-03-04 14:35                     ` Steven Price
2019-03-04 14:53                       ` Mike Rapoport
2019-03-01 11:49         ` Mike Rapoport
2019-03-01 12:28           ` Kirill A. Shutemov
2019-02-21 11:34 ` [PATCH v2 04/13] mm: pagewalk: Add p4d_entry() and pgd_entry() Steven Price
2019-02-21 11:34 ` [PATCH v2 05/13] mm: pagewalk: Allow walking without vma Steven Price
2019-02-21 11:34 ` [PATCH v2 06/13] mm: pagewalk: Add 'depth' parameter to pte_hole Steven Price
2019-02-21 11:34 ` [PATCH v2 07/13] mm: pagewalk: Add test_p?d callbacks Steven Price
2019-02-21 11:34 ` [PATCH v2 08/13] arm64: mm: Convert mm/dump.c to use walk_page_range() Steven Price
2019-02-21 11:34 ` [PATCH v2 09/13] x86/mm: Point to struct seq_file from struct pg_state Steven Price
2019-02-21 11:34 ` [PATCH v2 10/13] x86/mm+efi: Convert ptdump_walk_pgd_level() to take a mm_struct Steven Price
2019-02-21 11:35 ` [PATCH v2 11/13] x86/mm: Convert ptdump_walk_pgd_level_debugfs() to take an mm_struct Steven Price
2019-02-21 11:35 ` [PATCH v2 12/13] x86/mm: Convert ptdump_walk_pgd_level_core() " Steven Price
2019-02-21 11:35 ` [PATCH v2 13/13] x86: mm: Convert dump_pagetables to use walk_page_range Steven Price

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=20190303071253.GA7585@rapoport-lnx \
    --to=rppt@linux.ibm.com \
    --cc=Mark.Rutland@arm.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=james.morse@arm.com \
    --cc=jglisse@redhat.com \
    --cc=kan.liang@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=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=steven.price@arm.com \
    --cc=tglx@linutronix.de \
    --cc=will.deacon@arm.com \
    --cc=x86@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).