lkml.org 
[lkml]   [2005]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 07/10] uml: avoid fixing faults while atomic
    "Paolo 'Blaisorblade' Giarrusso" <blaisorblade@yahoo.it> wrote:
    >
    > From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
    >
    > Following i386, we should maybe refuse trying to fault in pages when we're
    > doing atomic operations, because to handle the fault we could need to take
    > already taken spinlocks.
    >
    > Also, if we're doing an atomic operation (in the sense of in_atomic()) we're
    > surely in kernel mode and we're surely going to handle adequately the failed
    > fault, so it's safe to behave this way.
    >
    > Currently, on UML SMP is rarely used, and we don't support PREEMPT, so this is
    > unlikely to create problems right now, but it might in the future.
    >

    That's not really an accurate explanation/understanding of what's going on
    in there.

    There's an extremely special-case in the pagefault handlers where we fail
    the fault if in_atomic(). It's unrelated to spinlocks (spinlocks don't
    even cause in_atomic() to become true if !CONFIG_PREEMPT).

    It has to do with kmap_atomic(). There's tricksy code in mm/filemap.c
    which will fault the target page in by hand and will then take an atomic
    kmap and will then raise current->preempt_count by hand, so in_atomic()
    becomes true even if !COFNIG_PREEMPT.

    So at this stage we expect to be able to do a copy_to/from_user to/from
    pagecache without taking a fault, because we just faulted the page in by
    hand. And we're not allowed to take a fault, because we're holding an
    atomic kmap. But if we _do_ take a fault (extreme memory pressure, racing
    munmap, etc) then we want to fail the pagefault immediately.

    The in_atomic() test in x86's do_page_fault() is in fact a message passed
    into it from filemap.c's kmap_atomic(). It has accidental side-effects,
    such as making copy_to_user() fail if inside spinlocks when
    CONFIG_PREEMPT=y.


    So I think this change is only needed if UML implements kmap_atomic, as in
    arch/i386/mm/highmem.c, which it surely does not do?


    >
    > diff --git a/arch/um/kernel/trap_kern.c b/arch/um/kernel/trap_kern.c
    > --- a/arch/um/kernel/trap_kern.c
    > +++ b/arch/um/kernel/trap_kern.c
    > @@ -40,6 +40,12 @@ int handle_page_fault(unsigned long addr
    > int err = -EFAULT;
    >
    > *code_out = SEGV_MAPERR;
    > +
    > + /* If the fault was during atomic operation, don't take the fault, just
    > + * fail. */
    > + if (in_atomic())
    > + goto out_nosemaphore;
    > +
    > down_read(&mm->mmap_sem);
    > vma = find_vma(mm, address);
    > if(!vma)
    > @@ -90,6 +96,7 @@ survive:
    > flush_tlb_page(vma, address);
    > out:
    > up_read(&mm->mmap_sem);
    > +out_nosemaphore:
    > return(err);
    >
    > /*
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-09-21 21:53    [W:0.025 / U:36.180 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site