linux-kernel - Re: [PATCH] mm: remove unintentional voluntary preemption in get_mmap_lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHk-=wg25LEzeaDnGyj5KfGmujDuV+_96hCOhVx2ScOtLW40gQ@mail.gmail.com>
Date:   Mon, 21 Aug 2023 07:38:32 +0200
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Mateusz Guzik <mjguzik@...il.com>, akpm@...ux-foundation.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH] mm: remove unintentional voluntary preemption in get_mmap_lock_carefully

On Mon, 21 Aug 2023 at 06:55, Linus Torvalds
<torvalds@...ux-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