From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Linus Torvalds <torvalds@linux-foundation.org>
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: Sun, 27 Dec 2020 01:40:16 +0300 [thread overview]
Message-ID: <20201226224016.dxjmordcfj75xgte@box> (raw)
In-Reply-To: <CAHk-=wjesveWEQZ4tqRssSSQvuxx46LqYfME+uxKfghxAe6U_w@mail.gmail.com>
On Sat, Dec 26, 2020 at 01:16:09PM -0800, Linus Torvalds wrote:
> On Sat, Dec 26, 2020 at 1:04 PM Hugh Dickins <hughd@google.com> wrote:
> >
> >
> > Hold on. I guess this one will suffer from the same bug as the previous.
> > I was about to report back, after satisfactory overnight testing of that
> > version - provided that one big little bug is fixed:
> >
> > --- a/mm/filemap.c
> > +++ b/mm/filemap.c
> > @@ -2919,7 +2919,7 @@ static bool filemap_map_pmd(struct vm_fa
> >
> > if (pmd_none(*vmf->pmd) &&
> > PageTransHuge(page) &&
> > - do_set_pmd(vmf, page)) {
> > + do_set_pmd(vmf, page) == 0) {
> > unlock_page(page);
> > return true;
> > }
>
> I missed that entirely, because when just reading the patch it looks
> fine and I didn't look at what do_set_pmd() function returns outside
> the patch.
>
> And maybe it would be better to write it as
>
> if (pmd_none(*vmf->pmd) && PageTransHuge(page)) {
> vm_fault_t ret = do_set_pmd(vmf, page);
> if (!ret) {
> ...
>
> instead to make it a bit more explicit about how that return value is
> a vm_fault_t there...
>
> And see my other email about how I suspect there is still a leak in
> that patch for the previous test-case.
Ughh...
Here's the fixup I have so far. It doesn't blow up immediately, but please
take a closer look. Who knows what stupid mistake I did this time. :/
diff --git a/mm/filemap.c b/mm/filemap.c
index 3a92aaa59b9b..c4b374678e7d 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2837,16 +2837,21 @@ static bool filemap_map_pmd(struct vm_fault *vmf, struct page *page)
struct mm_struct *mm = vmf->vma->vm_mm;
/* Huge page is mapped? No need to proceed. */
- if (pmd_trans_huge(*vmf->pmd))
- return true;
-
- if (pmd_none(*vmf->pmd) &&
- PageTransHuge(page) &&
- do_set_pmd(vmf, page)) {
+ if (pmd_trans_huge(*vmf->pmd)) {
unlock_page(page);
+ put_page(page);
return true;
}
+ if (pmd_none(*vmf->pmd) && PageTransHuge(page)) {
+ vm_fault_t ret = do_set_pmd(vmf, page);
+ if (!ret) {
+ /* The page is mapped successfully, reference consumed. */
+ unlock_page(page);
+ return true;
+ }
+ }
+
if (pmd_none(*vmf->pmd)) {
vmf->ptl = pmd_lock(mm, vmf->pmd);
if (likely(pmd_none(*vmf->pmd))) {
@@ -2867,7 +2872,7 @@ static bool filemap_map_pmd(struct vm_fault *vmf, struct page *page)
return false;
}
-static struct page *next_stable_page(struct page *page, struct vm_fault *vmf,
+static struct page *next_uptodate_page(struct page *page, struct vm_fault *vmf,
struct xa_state *xas, pgoff_t end_pgoff)
{
struct address_space *mapping = vmf->vma->vm_file->f_mapping;
@@ -2914,15 +2919,16 @@ static inline struct page *first_map_page(struct vm_fault *vmf,
struct xa_state *xas,
pgoff_t end_pgoff)
{
- return next_stable_page(xas_find(xas, end_pgoff), vmf, xas, end_pgoff);
+ return next_uptodate_page(xas_find(xas, end_pgoff),
+ vmf, xas, end_pgoff);
}
static inline struct page *next_map_page(struct vm_fault *vmf,
struct xa_state *xas,
pgoff_t end_pgoff)
{
- return next_stable_page(xas_next_entry(xas, end_pgoff),
- vmf, xas, end_pgoff);
+ return next_uptodate_page(xas_next_entry(xas, end_pgoff),
+ vmf, xas, end_pgoff);
}
void filemap_map_pages(struct vm_fault *vmf,
--
Kirill A. Shutemov
next prev parent reply other threads:[~2020-12-26 22:41 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
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 [this message]
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=20201226224016.dxjmordcfj75xgte@box \
--to=kirill@shutemov.name \
--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=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=torvalds@linux-foundation.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).