All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Peter Xu <peterx@redhat.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org
Subject: Re: [PATCH 0/3] mm/arch: Fix a few collide definition on private use of VM_FAULT_*
Date: Mon, 6 Feb 2023 05:09:57 +0000	[thread overview]
Message-ID: <Y+CLpdnOGFg28uMJ@casper.infradead.org> (raw)
In-Reply-To: <Y+BxhuGUx1K+3XHb@xz-m1.local>

On Sun, Feb 05, 2023 at 10:18:30PM -0500, Peter Xu wrote:
> On Mon, Feb 06, 2023 at 02:51:18AM +0000, Matthew Wilcox wrote:
> > That wasn't what I meant.  I meant putting VM_FAULT_BADMAP and
> > VM_FAULT_SIGSEGV in mm_types.h.  Not having "Here is a range of reserved
> > arch private ones".
> 
> VM_FAULT_SIGSEGV is there already; I assume you meant adding them all
> directly into vm_fault_reason.
> 
> Then I don't think it's a good idea..
> 
> Currently vm_fault_reason is a clear interface for handle_mm_fault() for
> not only arch pffault handlers but also soft faults like GUP.
> 
> If handle_mm_fault() doesn't return VM_FAULT_BADMAP at all, I don't think
> we should have it as public API at all.  When arch1 people reading the
> VM_FAULT_ documents, it shouldn't care about some fault reason that only
> happens with arch2.  Gup shouldn't care about it either.
> 
> Logically a new page fault handler should handle all the retval of
> vm_fault_reason afaiu.  That shouldn't include e.g. VM_FAULT_BADMAP either.

Hmm, right.  Looking specifically at how s390 uses VM_FAULT_BADMAP,
it just seems to be a badly structured fault.c.  Seems to me that
do_fault_error() should take an extra si_code argument, and
instead of returning VM_FAULT_BADACCESS / VM_FAULT_BADMAP from
various functions, those functions should call do_fault_error()
directly, passing it VM_FAULT_SIGSEGV and the appropriate si_code.

But this is all on the s390 people to fix; I don't want to break their
arch by trying it myself.

WARNING: multiple messages have this Message-ID (diff)
From: Matthew Wilcox <willy@infradead.org>
To: Peter Xu <peterx@redhat.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org
Subject: Re: [PATCH 0/3] mm/arch: Fix a few collide definition on private use of VM_FAULT_*
Date: Mon, 6 Feb 2023 05:09:57 +0000	[thread overview]
Message-ID: <Y+CLpdnOGFg28uMJ@casper.infradead.org> (raw)
In-Reply-To: <Y+BxhuGUx1K+3XHb@xz-m1.local>

On Sun, Feb 05, 2023 at 10:18:30PM -0500, Peter Xu wrote:
> On Mon, Feb 06, 2023 at 02:51:18AM +0000, Matthew Wilcox wrote:
> > That wasn't what I meant.  I meant putting VM_FAULT_BADMAP and
> > VM_FAULT_SIGSEGV in mm_types.h.  Not having "Here is a range of reserved
> > arch private ones".
> 
> VM_FAULT_SIGSEGV is there already; I assume you meant adding them all
> directly into vm_fault_reason.
> 
> Then I don't think it's a good idea..
> 
> Currently vm_fault_reason is a clear interface for handle_mm_fault() for
> not only arch pffault handlers but also soft faults like GUP.
> 
> If handle_mm_fault() doesn't return VM_FAULT_BADMAP at all, I don't think
> we should have it as public API at all.  When arch1 people reading the
> VM_FAULT_ documents, it shouldn't care about some fault reason that only
> happens with arch2.  Gup shouldn't care about it either.
> 
> Logically a new page fault handler should handle all the retval of
> vm_fault_reason afaiu.  That shouldn't include e.g. VM_FAULT_BADMAP either.

Hmm, right.  Looking specifically at how s390 uses VM_FAULT_BADMAP,
it just seems to be a badly structured fault.c.  Seems to me that
do_fault_error() should take an extra si_code argument, and
instead of returning VM_FAULT_BADACCESS / VM_FAULT_BADMAP from
various functions, those functions should call do_fault_error()
directly, passing it VM_FAULT_SIGSEGV and the appropriate si_code.

But this is all on the s390 people to fix; I don't want to break their
arch by trying it myself.

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

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

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-05 23:17 [PATCH 0/3] mm/arch: Fix a few collide definition on private use of VM_FAULT_* Peter Xu
2023-02-05 23:17 ` Peter Xu
2023-02-05 23:17 ` [PATCH 1/3] mm/arm: Define private VM_FAULT_* reasons from top bits Peter Xu
2023-02-05 23:17   ` Peter Xu
2023-02-05 23:17 ` [PATCH 2/3] mm/arm64: " Peter Xu
2023-02-05 23:17   ` Peter Xu
2023-02-05 23:17 ` [PATCH 3/3] mm/s390: " Peter Xu
2023-02-05 23:17   ` Peter Xu
2023-02-09 20:10   ` Heiko Carstens
2023-02-09 20:10     ` Heiko Carstens
2023-02-06  0:10 ` [PATCH 0/3] mm/arch: Fix a few collide definition on private use of VM_FAULT_* Matthew Wilcox
2023-02-06  0:10   ` Matthew Wilcox
2023-02-06  0:54   ` Peter Xu
2023-02-06  0:54     ` Peter Xu
2023-02-06  2:51     ` Matthew Wilcox
2023-02-06  2:51       ` Matthew Wilcox
2023-02-06  3:18       ` Peter Xu
2023-02-06  3:18         ` Peter Xu
2023-02-06  5:09         ` Matthew Wilcox [this message]
2023-02-06  5:09           ` Matthew Wilcox
2023-02-09 20:04           ` Heiko Carstens
2023-02-09 20:04             ` Heiko Carstens

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=Y+CLpdnOGFg28uMJ@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=peterx@redhat.com \
    /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.