Re: [PATCH 4/7] maccess: Use a scoped guard for page faults
Muhammad Usama Anjum <[email protected]>
| Newsgroups | org.infradead.lists.linux-arm-kernel,org.kernel.vger.bpf,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On 25/08/2026 11:36 am, David Hildenbrand (Arm) wrote: > On 8/24/26 18:04, Muhammad Usama Anjum wrote: >> Kernel nofault copy and string paths open-code page-fault disable and >> enable around label-based loops, duplicating cleanup on success and >> failure. >> >> Use a page-fault scope guard instead. Leaving the scope now re-enables >> page faults on both paths without separate cleanup at the fault label. >> >> No functional change. >> >> Signed-off-by: Muhammad Usama Anjum <[email protected]> >> --- >> mm/maccess.c | 53 +++++++++++++++++++++++++--------------------------- >> 1 file changed, 25 insertions(+), 28 deletions(-) >> >> diff --git a/mm/maccess.c b/mm/maccess.c >> index c59a0e092d24a..f695ceefe6fcc 100644 >> --- a/mm/maccess.c >> +++ b/mm/maccess.c >> @@ -38,18 +38,17 @@ long copy_from_kernel_nofault(void *dst, const void *src, size_t size) >> if (!size) >> return 0; >> >> - pagefault_disable(); >> - if (!(align & 7)) >> - copy_from_kernel_nofault_loop(dst, src, size, u64, Efault); >> - if (!(align & 3)) >> - copy_from_kernel_nofault_loop(dst, src, size, u32, Efault); >> - if (!(align & 1)) >> - copy_from_kernel_nofault_loop(dst, src, size, u16, Efault); >> - copy_from_kernel_nofault_loop(dst, src, size, u8, Efault); >> - pagefault_enable(); >> + scoped_guard(pagefault) { >> + if (!(align & 7)) >> + copy_from_kernel_nofault_loop(dst, src, size, u64, Efault); >> + if (!(align & 3)) >> + copy_from_kernel_nofault_loop(dst, src, size, u32, Efault); >> + if (!(align & 1)) >> + copy_from_kernel_nofault_loop(dst, src, size, u16, Efault); >> + copy_from_kernel_nofault_loop(dst, src, size, u8, Efault); >> + } > > While I understand what pagefault_disable+pagefault_enable does, it is > completely unclear what scoped_guard(pagefault) does. > > Should this be scoped_guard(disabled_pagefaults) or sth like that? > The `pagefault` guard is already defined in `include/linux/uaccess.h`: DEFINE_LOCK_GUARD_0(pagefault, pagefault_disable(), pagefault_enable()) Most users use `guard(pagefault)()`, which calls `pagefault_disable()` in the start and `pagefault_enable()` when the enclosing scope is left. In case of guard, the scope ends at function exit. scopped_guard() on the other hand defines the scope with {} (probably called code block). Use of scoped_guard makes patch 5/7 straightforward: `scoped_guard(__kernel_nofault_bare)` is nested inside the pagefault scope. -- Thanks, Usama