lkml.org 
[lkml]   [2019]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 3/7] mm: Introduce FAULT_FLAG_INTERRUPTIBLE
    From
    Date
    On 05.09.19 12:15, Peter Xu wrote:
    > handle_userfaultfd() is currently the only one place in the kernel
    > page fault procedures that can respond to non-fatal userspace signals.
    > It was trying to detect such an allowance by checking against USER &
    > KILLABLE flags, which was "un-official".
    >
    > In this patch, we introduced a new flag (FAULT_FLAG_INTERRUPTIBLE) to
    > show that the fault handler allows the fault procedure to respond even
    > to non-fatal signals. Meanwhile, add this new flag to the default
    > fault flags so that all the page fault handlers can benefit from the
    > new flag. With that, replacing the userfault check to this one.
    >
    > Since the line is getting even longer, clean up the fault flags a bit
    > too to ease TTY users.
    >
    > Although we've got a new flag and applied it, we shouldn't have any
    > functional change with this patch so far.
    >
    > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
    > Signed-off-by: Peter Xu <peterx@redhat.com>
    > ---
    > fs/userfaultfd.c | 4 +---
    > include/linux/mm.h | 39 ++++++++++++++++++++++++++++-----------
    > 2 files changed, 29 insertions(+), 14 deletions(-)
    >
    > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
    > index ccbdbd62f0d8..4a8ad2dc2b6f 100644
    > --- a/fs/userfaultfd.c
    > +++ b/fs/userfaultfd.c
    > @@ -462,9 +462,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason)
    > uwq.ctx = ctx;
    > uwq.waken = false;
    >
    > - return_to_userland =
    > - (vmf->flags & (FAULT_FLAG_USER|FAULT_FLAG_KILLABLE)) ==
    > - (FAULT_FLAG_USER|FAULT_FLAG_KILLABLE);
    > + return_to_userland = vmf->flags & FAULT_FLAG_INTERRUPTIBLE;
    > blocking_state = return_to_userland ? TASK_INTERRUPTIBLE :
    > TASK_KILLABLE;
    >
    > diff --git a/include/linux/mm.h b/include/linux/mm.h
    > index 57fb5c535f8e..53ec7abb8472 100644
    > --- a/include/linux/mm.h
    > +++ b/include/linux/mm.h
    > @@ -383,22 +383,38 @@ extern unsigned int kobjsize(const void *objp);
    > */
    > extern pgprot_t protection_map[16];
    >
    > -#define FAULT_FLAG_WRITE 0x01 /* Fault was a write access */
    > -#define FAULT_FLAG_MKWRITE 0x02 /* Fault was mkwrite of existing pte */
    > -#define FAULT_FLAG_ALLOW_RETRY 0x04 /* Retry fault if blocking */
    > -#define FAULT_FLAG_RETRY_NOWAIT 0x08 /* Don't drop mmap_sem and wait when retrying */
    > -#define FAULT_FLAG_KILLABLE 0x10 /* The fault task is in SIGKILL killable region */
    > -#define FAULT_FLAG_TRIED 0x20 /* Second try */
    > -#define FAULT_FLAG_USER 0x40 /* The fault originated in userspace */
    > -#define FAULT_FLAG_REMOTE 0x80 /* faulting for non current tsk/mm */
    > -#define FAULT_FLAG_INSTRUCTION 0x100 /* The fault was during an instruction fetch */
    > +/**
    > + * Fault flag definitions.
    > + *
    > + * @FAULT_FLAG_WRITE: Fault was a write fault.
    > + * @FAULT_FLAG_MKWRITE: Fault was mkwrite of existing PTE.
    > + * @FAULT_FLAG_ALLOW_RETRY: Allow to retry the fault if blocked.
    > + * @FAULT_FLAG_RETRY_NOWAIT: Don't drop mmap_sem and wait when retrying.
    > + * @FAULT_FLAG_KILLABLE: The fault task is in SIGKILL killable region.
    > + * @FAULT_FLAG_TRIED: The fault has been tried once.
    > + * @FAULT_FLAG_USER: The fault originated in userspace.
    > + * @FAULT_FLAG_REMOTE: The fault is not for current task/mm.
    > + * @FAULT_FLAG_INSTRUCTION: The fault was during an instruction fetch.
    > + * @FAULT_FLAG_INTERRUPTIBLE: The fault can be interrupted by non-fatal signals.
    > + */
    > +#define FAULT_FLAG_WRITE 0x01
    > +#define FAULT_FLAG_MKWRITE 0x02
    > +#define FAULT_FLAG_ALLOW_RETRY 0x04
    > +#define FAULT_FLAG_RETRY_NOWAIT 0x08
    > +#define FAULT_FLAG_KILLABLE 0x10
    > +#define FAULT_FLAG_TRIED 0x20
    > +#define FAULT_FLAG_USER 0x40
    > +#define FAULT_FLAG_REMOTE 0x80
    > +#define FAULT_FLAG_INSTRUCTION 0x100
    > +#define FAULT_FLAG_INTERRUPTIBLE 0x200
    >

    I'd probably split off the unrelated doc changes. Just a matter of taste.

    > /*
    > * The default fault flags that should be used by most of the
    > * arch-specific page fault handlers.
    > */
    > #define FAULT_FLAG_DEFAULT (FAULT_FLAG_ALLOW_RETRY | \
    > - FAULT_FLAG_KILLABLE)
    > + FAULT_FLAG_KILLABLE | \
    > + FAULT_FLAG_INTERRUPTIBLE)

    So by default, all faults are marked interruptible, also
    !FAULT_FLAG_USER. I assume the trick right now is that
    handle_userfault() will indeed only be called on user faults and the
    flag is used nowhere else ;)

    Would it make sense to name it FAULT_FLAG_USER_INTERRUPTIBLE, to stress
    that the flag only applies to user faults? (or am I missing something
    and this could also apply to !user faults somewhen in the future?

    (I am no expert on the fault paths yet, so sorry for the silly questions)

    >
    > #define FAULT_FLAG_TRACE \
    > { FAULT_FLAG_WRITE, "WRITE" }, \
    > @@ -409,7 +425,8 @@ extern pgprot_t protection_map[16];
    > { FAULT_FLAG_TRIED, "TRIED" }, \
    > { FAULT_FLAG_USER, "USER" }, \
    > { FAULT_FLAG_REMOTE, "REMOTE" }, \
    > - { FAULT_FLAG_INSTRUCTION, "INSTRUCTION" }
    > + { FAULT_FLAG_INSTRUCTION, "INSTRUCTION" }, \
    > + { FAULT_FLAG_INTERRUPTIBLE, "INTERRUPTIBLE" }
    >
    > /*
    > * vm_fault is filled by the the pagefault handler and passed to the vma's
    >


    --

    Thanks,

    David / dhildenb

    \
     
     \ /
      Last update: 2019-09-06 11:03    [W:3.695 / U:0.304 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site