All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Peter Xu <peterx@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-arch@vger.kernel.org, linux-alpha@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-hexagon@vger.kernel.org,
	linux-m68k@lists.linux-m68k.org, Michal Simek <monstr@monstr.eu>,
	Dinh Nguyen <dinguyen@kernel.org>,
	openrisc@lists.librecores.org, linux-parisc@vger.kernel.org,
	linux-riscv@lists.infradead.org, sparclinux@vger.kernel.org
Subject: Re: [RFC][PATCHSET] VM_FAULT_RETRY fixes
Date: Sun, 5 Feb 2023 05:10:21 +0000	[thread overview]
Message-ID: <Y986PWjDy9YbHBEw@ZenIV> (raw)
In-Reply-To: <Y92mP1GT28KfnPEQ@ZenIV>

On Sat, Feb 04, 2023 at 12:26:39AM +0000, Al Viro wrote:
> On Thu, Feb 02, 2023 at 05:56:22PM -0500, Peter Xu wrote:
> 
> > IMHO it'll be merely impossible to merge things across most (if not to say,
> > all) archs.  It will need to be start from one or at least a few that still
> > shares a major common base - I would still rely on x86 as a start - then we
> > try to use the helper in as much archs as possible.
> > 
> > Even on x86, I do also see challenges so I'm not sure whether a common
> > enough routine can be abstracted indeed.  But I believe there's a way to do
> > this because obviously we still see tons of duplicated logics falling
> > around.  It may definitely need time to think out where's the best spot to
> > start, and how to gradually move towards covering more archs starting from
> > one.
> 
> FWIW, after going through everything from alpha to loongarch (in alphabetic
> order, skipping the itanic) the following seems to be suitable for all of
> them:
> 
> generic_fault(address, flags, vm_flags, regs)
[snip]

After looking through other architectures: that needs changes.

AFAICS, the right approach would be to add a pointer to (uninitialized)
kernel_siginfo.  And let it pass the signal number, etc. through that.
That way all "we want to raise a signal" return values fold together.
*IF* the page fault wants to do something extra on SIGSEGV, but not on
SIGBUS (we have several such), it can key that upon info.si_signo.

Speaking of SIGSEGV, there's a fun situation with VM_FAULT_SIGSEGV:

1) Only x86 and ppc generate VM_FAULT_SIGSEGV in handle_mm_fault()
without hitting WARN_ON_ONCE().  And neither actually returns it
to page fault handler - the conditions that would've led to that
return value (pkey stuff) are checked in the page fault handler
and handle_mm_fault() is not called in such cases.

2) on alpha, hexagon, itanic, loongarch, microblaze, mips, nios2,
openrisc, sparc, um and xtensa #PF handler would end up with SEGV_ACCERR
if it saw VM_FAULT_SIGNAL.

3) on arm, arm64, arc, m68k, powerpc, s390, sh and x86 - SEGV_MAPERR
(except that neither x86 nor powerpc #PF ever see it)

4) on csky and riscv it would end up with BUG()

5) on parisc it's complicated^Wbroken - it tries to decide which
one to use after having unlocked mmap and looking at vma in process.

As it is, VM_FAULT_SIGSEGV had ended up as "we need to report some
error to get_user_pages() and similar callers in cases when
page fault handler would've detected pkey problem and refused
to call handle_mm_fault()", with several places where it's
"we had been called with bogus arguments; printk and return
something to indicate the things had gone wrong".  It used to
have other uses, but this is what it had become now - grep and
you'll see.

AFAICS, all checks for it in page fault handlers are pretty much
dead code...

WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@zeniv.linux.org.uk>
To: Peter Xu <peterx@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-arch@vger.kernel.org, linux-alpha@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-hexagon@vger.kernel.org,
	linux-m68k@lists.linux-m68k.org, Michal Simek <monstr@monstr.eu>,
	Dinh Nguyen <dinguyen@kernel.org>,
	openrisc@lists.librecores.org, linux-parisc@vger.kernel.org,
	linux-riscv@lists.infradead.org, sparclinux@vger.kernel.org
Subject: Re: [RFC][PATCHSET] VM_FAULT_RETRY fixes
Date: Sun, 5 Feb 2023 05:10:21 +0000	[thread overview]
Message-ID: <Y986PWjDy9YbHBEw@ZenIV> (raw)
In-Reply-To: <Y92mP1GT28KfnPEQ@ZenIV>

On Sat, Feb 04, 2023 at 12:26:39AM +0000, Al Viro wrote:
> On Thu, Feb 02, 2023 at 05:56:22PM -0500, Peter Xu wrote:
> 
> > IMHO it'll be merely impossible to merge things across most (if not to say,
> > all) archs.  It will need to be start from one or at least a few that still
> > shares a major common base - I would still rely on x86 as a start - then we
> > try to use the helper in as much archs as possible.
> > 
> > Even on x86, I do also see challenges so I'm not sure whether a common
> > enough routine can be abstracted indeed.  But I believe there's a way to do
> > this because obviously we still see tons of duplicated logics falling
> > around.  It may definitely need time to think out where's the best spot to
> > start, and how to gradually move towards covering more archs starting from
> > one.
> 
> FWIW, after going through everything from alpha to loongarch (in alphabetic
> order, skipping the itanic) the following seems to be suitable for all of
> them:
> 
> generic_fault(address, flags, vm_flags, regs)
[snip]

After looking through other architectures: that needs changes.

AFAICS, the right approach would be to add a pointer to (uninitialized)
kernel_siginfo.  And let it pass the signal number, etc. through that.
That way all "we want to raise a signal" return values fold together.
*IF* the page fault wants to do something extra on SIGSEGV, but not on
SIGBUS (we have several such), it can key that upon info.si_signo.

Speaking of SIGSEGV, there's a fun situation with VM_FAULT_SIGSEGV:

1) Only x86 and ppc generate VM_FAULT_SIGSEGV in handle_mm_fault()
without hitting WARN_ON_ONCE().  And neither actually returns it
to page fault handler - the conditions that would've led to that
return value (pkey stuff) are checked in the page fault handler
and handle_mm_fault() is not called in such cases.

2) on alpha, hexagon, itanic, loongarch, microblaze, mips, nios2,
openrisc, sparc, um and xtensa #PF handler would end up with SEGV_ACCERR
if it saw VM_FAULT_SIGNAL.

3) on arm, arm64, arc, m68k, powerpc, s390, sh and x86 - SEGV_MAPERR
(except that neither x86 nor powerpc #PF ever see it)

4) on csky and riscv it would end up with BUG()

5) on parisc it's complicated^Wbroken - it tries to decide which
one to use after having unlocked mmap and looking at vma in process.

As it is, VM_FAULT_SIGSEGV had ended up as "we need to report some
error to get_user_pages() and similar callers in cases when
page fault handler would've detected pkey problem and refused
to call handle_mm_fault()", with several places where it's
"we had been called with bogus arguments; printk and return
something to indicate the things had gone wrong".  It used to
have other uses, but this is what it had become now - grep and
you'll see.

AFAICS, all checks for it in page fault handlers are pretty much
dead code...

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@zeniv.linux.org.uk>
To: Peter Xu <peterx@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-arch@vger.kernel.org, linux-alpha@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-hexagon@vger.kernel.org,
	linux-m68k@lists.linux-m68k.org, Michal Simek <monstr@monstr.eu>,
	Dinh Nguyen <dinguyen@kernel.org>,
	openrisc@lists.librecores.org, linux-parisc@vger.kernel.org,
	linux-riscv@lists.infradead.org, sparclinux@vger.kernel.org
Subject: Re: [RFC][PATCHSET] VM_FAULT_RETRY fixes
Date: Sun, 05 Feb 2023 05:10:21 +0000	[thread overview]
Message-ID: <Y986PWjDy9YbHBEw@ZenIV> (raw)
In-Reply-To: <Y92mP1GT28KfnPEQ@ZenIV>

On Sat, Feb 04, 2023 at 12:26:39AM +0000, Al Viro wrote:
> On Thu, Feb 02, 2023 at 05:56:22PM -0500, Peter Xu wrote:
> 
> > IMHO it'll be merely impossible to merge things across most (if not to say,
> > all) archs.  It will need to be start from one or at least a few that still
> > shares a major common base - I would still rely on x86 as a start - then we
> > try to use the helper in as much archs as possible.
> > 
> > Even on x86, I do also see challenges so I'm not sure whether a common
> > enough routine can be abstracted indeed.  But I believe there's a way to do
> > this because obviously we still see tons of duplicated logics falling
> > around.  It may definitely need time to think out where's the best spot to
> > start, and how to gradually move towards covering more archs starting from
> > one.
> 
> FWIW, after going through everything from alpha to loongarch (in alphabetic
> order, skipping the itanic) the following seems to be suitable for all of
> them:
> 
> generic_fault(address, flags, vm_flags, regs)
[snip]

After looking through other architectures: that needs changes.

AFAICS, the right approach would be to add a pointer to (uninitialized)
kernel_siginfo.  And let it pass the signal number, etc. through that.
That way all "we want to raise a signal" return values fold together.
*IF* the page fault wants to do something extra on SIGSEGV, but not on
SIGBUS (we have several such), it can key that upon info.si_signo.

Speaking of SIGSEGV, there's a fun situation with VM_FAULT_SIGSEGV:

1) Only x86 and ppc generate VM_FAULT_SIGSEGV in handle_mm_fault()
without hitting WARN_ON_ONCE().  And neither actually returns it
to page fault handler - the conditions that would've led to that
return value (pkey stuff) are checked in the page fault handler
and handle_mm_fault() is not called in such cases.

2) on alpha, hexagon, itanic, loongarch, microblaze, mips, nios2,
openrisc, sparc, um and xtensa #PF handler would end up with SEGV_ACCERR
if it saw VM_FAULT_SIGNAL.

3) on arm, arm64, arc, m68k, powerpc, s390, sh and x86 - SEGV_MAPERR
(except that neither x86 nor powerpc #PF ever see it)

4) on csky and riscv it would end up with BUG()

5) on parisc it's complicated^Wbroken - it tries to decide which
one to use after having unlocked mmap and looking at vma in process.

As it is, VM_FAULT_SIGSEGV had ended up as "we need to report some
error to get_user_pages() and similar callers in cases when
page fault handler would've detected pkey problem and refused
to call handle_mm_fault()", with several places where it's
"we had been called with bogus arguments; printk and return
something to indicate the things had gone wrong".  It used to
have other uses, but this is what it had become now - grep and
you'll see.

AFAICS, all checks for it in page fault handlers are pretty much
dead code...

  reply	other threads:[~2023-02-05  5:10 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-31 20:02 [RFC][PATCHSET] VM_FAULT_RETRY fixes Al Viro
2023-01-31 20:02 ` Al Viro
2023-01-31 20:03 ` [PATCH 01/10] alpha: fix livelock in uaccess Al Viro
2023-01-31 20:03   ` Al Viro
2023-03-07  0:48   ` patchwork-bot+linux-riscv
2023-03-07  0:48     ` patchwork-bot+linux-riscv
2023-01-31 20:03 ` [PATCH 02/10] hexagon: " Al Viro
2023-01-31 20:03   ` Al Viro
2023-02-10  2:59   ` Brian Cain
2023-02-10  2:59     ` Brian Cain
2023-01-31 20:04 ` [PATCH 03/10] ia64: " Al Viro
2023-01-31 20:04   ` Al Viro
2023-01-31 20:04 ` [PATCH 04/10] m68k: " Al Viro
2023-01-31 20:04   ` Al Viro
2023-02-05  6:18   ` Finn Thain
2023-02-05  6:18     ` Finn Thain
2023-02-05  6:18     ` Finn Thain
2023-02-05 18:51     ` Linus Torvalds
2023-02-05 18:51       ` Linus Torvalds
2023-02-05 18:51       ` Linus Torvalds
2023-02-07  3:07       ` Finn Thain
2023-02-07  3:07         ` Finn Thain
2023-02-07  3:07         ` Finn Thain
2023-02-05 20:39     ` Al Viro
2023-02-05 20:39       ` Al Viro
2023-02-05 20:39       ` Al Viro
2023-02-05 20:41       ` Linus Torvalds
2023-02-05 20:41         ` Linus Torvalds
2023-02-05 20:41         ` Linus Torvalds
2023-02-06 12:08   ` Geert Uytterhoeven
2023-02-06 12:08     ` Geert Uytterhoeven
2023-02-06 12:08     ` Geert Uytterhoeven
2023-01-31 20:05 ` [PATCH 05/10] microblaze: " Al Viro
2023-01-31 20:05   ` Al Viro
2023-01-31 20:05 ` [PATCH 06/10] nios2: " Al Viro
2023-01-31 20:05   ` Al Viro
2023-01-31 20:06 ` [PATCH 07/10] openrisc: " Al Viro
2023-01-31 20:06   ` Al Viro
2023-01-31 20:06 ` [PATCH 08/10] parisc: " Al Viro
2023-01-31 20:06   ` Al Viro
2023-02-06 16:58   ` Helge Deller
2023-02-06 16:58     ` Helge Deller
2023-02-06 16:58     ` Helge Deller
2023-02-28 17:34     ` Al Viro
2023-02-28 17:34       ` Al Viro
2023-02-28 18:26       ` Helge Deller
2023-02-28 19:14         ` Al Viro
2023-02-28 19:32           ` Helge Deller
2023-02-28 20:00             ` Helge Deller
2023-02-28 20:22               ` Helge Deller
2023-02-28 22:57                 ` Al Viro
2023-03-01  4:00                   ` Helge Deller
2023-03-02 17:53                     ` Al Viro
2023-02-28 15:22   ` Guenter Roeck
2023-02-28 15:22     ` Guenter Roeck
2023-02-28 15:22     ` Guenter Roeck
2023-02-28 19:18     ` Michael Schmitz
2023-02-28 19:18       ` Michael Schmitz
2023-02-28 19:18       ` Michael Schmitz
2023-01-31 20:06 ` [PATCH 09/10] riscv: " Al Viro
2023-01-31 20:06   ` Al Viro
2023-02-06 20:06   ` Björn Töpel
2023-02-06 20:06     ` Björn Töpel
2023-02-06 20:06     ` Björn Töpel
2023-02-07 16:11   ` Geert Uytterhoeven
2023-02-07 16:11     ` Geert Uytterhoeven
2023-02-07 16:11     ` Geert Uytterhoeven
2023-01-31 20:07 ` [PATCH 10/10] sparc: " Al Viro
2023-01-31 20:07   ` Al Viro
2023-01-31 20:24 ` [RFC][PATCHSET] VM_FAULT_RETRY fixes Linus Torvalds
2023-01-31 20:24   ` Linus Torvalds
2023-01-31 20:24   ` Linus Torvalds
2023-01-31 21:10   ` Al Viro
2023-01-31 21:10     ` Al Viro
2023-01-31 21:19     ` Linus Torvalds
2023-01-31 21:19       ` Linus Torvalds
2023-01-31 21:19       ` Linus Torvalds
2023-01-31 21:49       ` Al Viro
2023-01-31 21:49         ` Al Viro
2023-02-01  0:00         ` Linus Torvalds
2023-02-01  0:00           ` Linus Torvalds
2023-02-01  0:00           ` Linus Torvalds
2023-02-01 19:48           ` Peter Xu
2023-02-01 19:48             ` Peter Xu
2023-02-01 19:48             ` Peter Xu
2023-02-01 22:18             ` Al Viro
2023-02-01 22:18               ` Al Viro
2023-02-01 22:18               ` Al Viro
2023-02-02  0:57               ` Al Viro
2023-02-02  0:57                 ` Al Viro
2023-02-02  0:57                 ` Al Viro
2023-02-02 22:56               ` Peter Xu
2023-02-02 22:56                 ` Peter Xu
2023-02-02 22:56                 ` Peter Xu
2023-02-04  0:26                 ` Al Viro
2023-02-04  0:26                   ` Al Viro
2023-02-04  0:26                   ` Al Viro
2023-02-05  5:10                   ` Al Viro [this message]
2023-02-05  5:10                     ` Al Viro
2023-02-05  5:10                     ` Al Viro
2023-02-04  0:47         ` [loongarch oddities] " Al Viro
2023-02-01  8:21       ` Helge Deller
2023-02-01  8:21         ` Helge Deller
2023-02-01  8:21         ` Helge Deller
2023-02-01 19:51         ` Linus Torvalds
2023-02-01 19:51           ` Linus Torvalds
2023-02-01 19:51           ` Linus Torvalds
2023-02-02  6:58       ` Al Viro
2023-02-02  6:58         ` Al Viro
2023-02-02  8:54         ` Michael Cree
2023-02-02  9:56           ` John Paul Adrian Glaubitz
2023-02-02 15:20           ` Al Viro
2023-02-02 20:20             ` Al Viro
2023-02-02 20:34         ` Linus Torvalds
2023-02-01 10:50 ` Mark Rutland
2023-02-01 10:50   ` Mark Rutland
2023-02-01 10:50   ` Mark Rutland
2023-02-06 12:08   ` Geert Uytterhoeven
2023-02-06 12:08     ` Geert Uytterhoeven
2023-02-06 12:08     ` Geert Uytterhoeven

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=Y986PWjDy9YbHBEw@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=dinguyen@kernel.org \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-hexagon@vger.kernel.org \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=monstr@monstr.eu \
    --cc=openrisc@lists.librecores.org \
    --cc=peterx@redhat.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=torvalds@linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.