From: David Hildenbrand <david@redhat.com>
To: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Cc: Mike Kravetz <mike.kravetz@oracle.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux MM <linux-mm@kvack.org>, stable <stable@vger.kernel.org>,
linux-s390 <linux-s390@vger.kernel.org>,
Heiko Carstens <hca@linux.ibm.com>
Subject: Re: [PATCH v2 2/2] mm/hugetlb: support write-faults in shared mappings
Date: Tue, 16 Aug 2022 22:43:37 +0200 [thread overview]
Message-ID: <CADFyXm5m1a+ZRwp1Kejt0L4HFcVBSoSz6mG-19_65CnR7s7Q-A@mail.gmail.com> (raw)
In-Reply-To: <20220816113359.33843f54@thinkpad>
Hi Gerald,
>
> Thanks, we were also trying to reproduce on x86, w/o success so far. But
> I guess that matches David latest observations wrt to our exception handling
> code on s390.
>
> Good news is that the problem goes away when I add this simple patch, which
> should result in proper VM_WRITE check for vma flags, before triggering a
> FAULT_FLAG_WRITE fault:
>
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -379,7 +379,9 @@ static inline vm_fault_t do_exception(struct pt_regs *regs, int access)
> flags = FAULT_FLAG_DEFAULT;
> if (user_mode(regs))
> flags |= FAULT_FLAG_USER;
> - if (access == VM_WRITE || is_write)
> + if (is_write)
> + access = VM_WRITE;
> + if (access == VM_WRITE)
> flags |= FAULT_FLAG_WRITE;
> mmap_read_lock(mm);
That's what I had in mind, good.
>
> Still find it a bit hard to believe that this > 10 years old logic really
> is/was broken all the time. I guess it simply did not matter for normal
> PTE faults, probably because the common fault handling code later would
> check itself via maybe_mkwrite(). And for hugetlb PTEs, it might not have
> mattered before commit bcd51a3c679d.
It is akward, but maybe we never really noticed for hugetlb (not sure
how common read-only mappings are after all).
>
> >
> > bcd51a3c679d eliminates the copying of page tables at fork for non-anon
> > hugetlb vmas. So, in these tests you would likely see more pte_none()
> > faults.
>
> Yes, makes sense, assuming now that it actually is related to s390
> exception handling code, not checking for VM_WRITE before triggering a
> write fault for pte_none().
>
> Thanks for checking! And Thanks a lot to David for finding that issue
> in s390 exception handling code!
Thanks! Looks like adding the WARN_ON_ONCE was the right decision.
prev parent reply other threads:[~2022-08-16 20:44 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-11 10:34 [PATCH v2 0/2] mm/hugetlb: fix write-fault handling for shared mappings David Hildenbrand
2022-08-11 10:34 ` [PATCH v2 1/2] mm/hugetlb: fix hugetlb not supporting softdirty tracking David Hildenbrand
2022-08-11 18:27 ` Mike Kravetz
2022-08-11 10:34 ` [PATCH v2 2/2] mm/hugetlb: support write-faults in shared mappings David Hildenbrand
2022-08-11 13:59 ` Peter Xu
2022-08-11 16:24 ` David Hildenbrand
2022-08-11 18:59 ` Mike Kravetz
2022-08-15 13:35 ` Gerald Schaefer
2022-08-15 15:07 ` David Hildenbrand
2022-08-15 15:59 ` Gerald Schaefer
2022-08-15 18:03 ` David Hildenbrand
2022-08-15 18:38 ` Gerald Schaefer
2022-08-15 21:43 ` Mike Kravetz
2022-08-16 9:33 ` Gerald Schaefer
2022-08-16 20:43 ` David Hildenbrand [this message]
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=CADFyXm5m1a+ZRwp1Kejt0L4HFcVBSoSz6mG-19_65CnR7s7Q-A@mail.gmail.com \
--to=david@redhat.com \
--cc=gerald.schaefer@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-s390@vger.kernel.org \
--cc=mike.kravetz@oracle.com \
--cc=stable@vger.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 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.