linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Suren Baghdasaryan <surenb@google.com>
Cc: akpm@linux-foundation.org, regressions@leemhuis.info,
	bagasdotme@gmail.com, jacobly.alt@gmail.com, willy@infradead.org,
	liam.howlett@oracle.com, david@redhat.com, peterx@redhat.com,
	ldufour@linux.ibm.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, gregkh@linuxfoundation.org,
	regressions@lists.linux.dev, "Jiri Slaby" <jirislaby@kernel.org>,
	"Holger Hoffstätte" <holger@applied-asynchrony.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking
Date: Sat, 8 Jul 2023 14:18:29 -0700	[thread overview]
Message-ID: <CAHk-=whhXFQj0Vwzh7bnjnLs=SSTsxyiY6jeb7ovOGnCes4aWg@mail.gmail.com> (raw)
In-Reply-To: <20230708191212.4147700-3-surenb@google.com>

On Sat, 8 Jul 2023 at 12:12, Suren Baghdasaryan <surenb@google.com> wrote:
>
>  kernel/fork.c | 1 +
>  1 file changed, 1 insertion(+)

I ended up editing your explanation a lot.

I'm not convinced that the bug has much to do with the delayed tlb flushing.

I think it's more fundamental than some tlb coherence issue: our VM
copying simply expects to not have any unrelated concurrent page fault
activity, and various random internal data structures simply rely on
that.

I made up an example that I'm not sure is relevant to any of the
particular failures, but that I think is a non-TLB case: the parent
'vma->anon_vma' chain is copied by dup_mmap() in anon_vma_fork(), and
it's possible that the parent vma didn't have any anon_vma associated
with it at that point.

But a concurrent page fault to the same vma - even *before* the page
tables have been copied, and when the TLB is still entirely coherent -
could then cause a anon_vma_prepare() on that parent vma, and
associate one of the pages with that anon-vma.

Then the page table copy happens, and that page gets marked read-only
again, and is added to both the parent and the child vma's, but the
child vma never got associated with the parents new anon_vma, because
it didn't exist when anon_vma_fork() happened.

Does this ever happen? I have no idea. But it would seem to be an
example that really has nothing to do with any TLB state, and is just
simply "we cannot handle concurrent page faults while we're busy
copying the mm".

Again - maybe I messed up, but it really feels like the missing
vma_start_write() was more fundamental, and not some "TLB coherency"
issue.

            Linus


  parent reply	other threads:[~2023-07-08 21:18 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-08 19:12 [PATCH v2 1/3] mm: lock a vma before stack expansion Suren Baghdasaryan
2023-07-08 19:12 ` [PATCH v2 2/3] mm: lock newly mapped VMA which can be modified after it becomes visible Suren Baghdasaryan
2023-07-08 19:12 ` [PATCH v2 3/3] fork: lock VMAs of the parent process when forking Suren Baghdasaryan
2023-07-08 19:22   ` Suren Baghdasaryan
2023-07-08 21:18   ` Linus Torvalds [this message]
2023-07-08 22:36     ` Suren Baghdasaryan
2023-07-08 22:53       ` Linus Torvalds
2023-07-08 23:03         ` Suren Baghdasaryan
2023-08-04 21:46   ` Mateusz Guzik
2023-08-04 22:49     ` Linus Torvalds
2023-08-04 23:25       ` Mateusz Guzik
2023-08-05  0:14         ` Linus Torvalds
2023-08-05  0:26           ` Suren Baghdasaryan
2023-08-05  0:34             ` Suren Baghdasaryan
2023-08-05  0:49               ` Mateusz Guzik
2023-08-05  1:06                 ` Suren Baghdasaryan
2023-08-05  1:16                   ` Mateusz Guzik
2023-08-05  1:36                     ` Suren Baghdasaryan
2023-08-05  1:06           ` Mateusz Guzik
2023-08-05  1:42             ` Suren Baghdasaryan
2023-08-09 21:07               ` Mateusz Guzik
2023-08-10 20:31                 ` Suren Baghdasaryan

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='CAHk-=whhXFQj0Vwzh7bnjnLs=SSTsxyiY6jeb7ovOGnCes4aWg@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=bagasdotme@gmail.com \
    --cc=david@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=holger@applied-asynchrony.com \
    --cc=jacobly.alt@gmail.com \
    --cc=jirislaby@kernel.org \
    --cc=ldufour@linux.ibm.com \
    --cc=liam.howlett@oracle.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=peterx@redhat.com \
    --cc=regressions@leemhuis.info \
    --cc=regressions@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    --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