lkml.org 
[lkml]   [2021]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 0/1] mm: restore full accuracy in COW page reuse
    On Mon, Jan 11, 2021 at 11:19 AM Linus Torvalds
    <torvalds@linux-foundation.org> wrote:
    >
    > On Sun, Jan 10, 2021 at 11:27 PM John Hubbard <jhubbard@nvidia.com> wrote:
    > > IMHO, a lot of the bits in page _refcount are still being wasted (even
    > > after GUP_PIN_COUNTING_BIAS overloading), because it's unlikely that
    > > there are many callers of gup/pup per page.
    >
    > It may be unlikely under real loads.
    >
    > But we've actually had overflow issues on this because rather than
    > real loads you can do attack loads (ie "lots of processes, lots of
    > pipe file descriptors, lots of vmsplice() operations on the same
    > page".
    >
    > We had to literally add that conditional "try_get_page()" that
    > protects against overflow..

    Actually, what I think might be a better model is to actually
    strengthen the rules even more, and get rid of GUP_PIN_COUNTING_BIAS
    entirely.

    What we could do is just make a few clear rules explicit (most of
    which we already basically hold to). Starting from that basic

    (a) Anonymous pages are made writable (ie COW) only when they have a
    page_count() of 1

    That very simple rule then automatically results in the corollary

    (b) a writable page in a COW mapping always starts out reachable
    _only_ from the page tables

    and now we could have a couple of really simple new rules:

    (c) we never ever make a writable page in a COW mapping read-only
    _unless_ it has a page_count() of 1

    (d) we never create a swap cache page out of a writable COW mapping page

    Now, if you combine these rules, the whole need for the
    GUP_PIN_COUNTING_BIAS basically goes away.

    Why? Because we know that the _only_ thing that can elevate the
    refcount of a writable COW page is GUP - we'll just make sure nothing
    else touches it.

    The whole "optimistic page references throigh page cache" etc are
    complete non-issues, because the whole point is that we already know
    it's not a page cache page. There is simply no other way to reach that
    page than through GUP.

    Ergo: any writable pte in a COW mapping that has a page with a
    page_count() > 1 is pinned by definition, and thus our

    page_maybe_dma_pinned(page)

    could remove that "maybe" part, and simply check for

    page_count(page) > 1

    (although the rule would be that this is only valid for a cow_mapping
    pte, and only while holding the page table lock! So maybe it would be
    good to pass in the vma and have an assert for that lock too).

    And the thing is, none of the above rules are complicated.

    The only new one would be the requirement that you cannot add a page
    to the swap cache unless it is read-only in the page tables. That may
    be the biggest hurdle here.

    The way we handle swap cache is that we *first* add it to the swap
    cache, and then we do a "try_to_unmap()" on it. So we currently don't
    actually try to walk the page tables until we have already done that
    swap cache thing.

    But I do think that the only major problem spot is that
    shrink_page_list() -> add_to_swap() case, and I think we could make
    add_to_swap() just do the rmap walk and turn it read-only first.

    (And it's worth pointing out that I'm only talking about regular
    non-huge pages above, the rules for splitting hugepages may impact
    those cases differently, I didn't really even try to think about those
    cases).

    But thatadd_to_swap() case might make it too painful. It _would_
    simplify our rules wrt anonymous mappings enormously, though.

    Linus

    \
     
     \ /
      Last update: 2021-01-11 23:20    [W:5.359 / U:0.128 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site