From: Linus Torvalds <torvalds@linux-foundation.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: Mateusz Guzik <mjguzik@gmail.com>,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
Subject: Re: [PATCH] mm: remove unintentional voluntary preemption in get_mmap_lock_carefully
Date: Mon, 21 Aug 2023 07:38:32 +0200 [thread overview]
Message-ID: <CAHk-=wg25LEzeaDnGyj5KfGmujDuV+_96hCOhVx2ScOtLW40gQ@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=win-keZbx6GFC4Q6VXUiFLfWxVDqcAUoV2A38rN29H5Xw@mail.gmail.com>
On Mon, 21 Aug 2023 at 06:55, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I do think that maybe we should then re-introduce the might_sleep() in
> some actually appropriate place in the page fault path, which might be
> 'handle_mm_fault()'.
Just to clarify: if we do it in handle_mm_fault(), we should obviously
do it only for kernel faults - just to avoid the unnecessary
preemption with lock held for the normal user space case.
That said, I don't love the handle_mm_fault() option, I just think it
would be lovely if we had that test in a generic place. Sadly, we
don't seem to have any such thing other than handle_mm_fault().
The reason we shouldn't do this for user space faults are fine is
because not only are they obviously always sleepable, but they also
reschedule on return to user space. So neither the debugging nor the
preemption makes sense there.
That's also why the per-vma locking paths don't need this - we only do
per-vma locking for user space faults.
So it really is only "kernel faults" it makes sense for, and they are
rare enough that I think the preemption point issue might be moot. But
it might still be better to just do this all as a special kernel fault
case in the exception handler - even if it's then
architecture-specific (like it always was before commit c2508ec5a58d).
Linus
prev parent reply other threads:[~2023-08-21 5:38 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-20 10:43 Mateusz Guzik
2023-08-20 11:36 ` Matthew Wilcox
2023-08-20 12:41 ` Mateusz Guzik
2023-08-20 12:46 ` Mateusz Guzik
2023-08-20 12:47 ` Linus Torvalds
2023-08-20 12:59 ` Linus Torvalds
2023-08-20 13:08 ` Mateusz Guzik
2023-08-20 13:00 ` Mateusz Guzik
2023-08-20 18:12 ` Matthew Wilcox
2023-08-21 1:13 ` Mateusz Guzik
2023-08-21 3:58 ` Matthew Wilcox
2023-08-21 4:55 ` Linus Torvalds
2023-08-21 5:38 ` Linus Torvalds [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='CAHk-=wg25LEzeaDnGyj5KfGmujDuV+_96hCOhVx2ScOtLW40gQ@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mjguzik@gmail.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