lkml.org 
[lkml]   [2019]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH v9 0/8] stg mail -e --version=v9 \
    From
    Date
    On 12.09.19 09:16, Michal Hocko wrote:
    > On Wed 11-09-19 18:09:18, David Hildenbrand wrote:
    >> On 11.09.19 15:51, Michal Hocko wrote:
    >>> On Wed 11-09-19 15:20:02, Michal Hocko wrote:
    >>> [...]
    >>>>> 4. Continuously report, not the "one time report everything" approach.
    >>>>
    >>>> So you mean the allocator reporting this rather than an external code to
    >>>> poll right? I do not know, how much this is nice to have than must have?
    >>>
    >>> Another idea that I haven't really thought through so it might turned
    >>> out to be completely bogus but let's try anyway. Your "report everything"
    >>> just made me look and realize that free_pages_prepare already performs
    >>> stuff that actually does something similar yet unrelated.
    >>>
    >>> We do report to special page poisoning, zeroying or
    >>> CONFIG_DEBUG_PAGEALLOC to unmap the address from the kernel address
    >>> space. This sounds like something fitting your model no?
    >>>
    >>
    >> AFAIKS, the poisoning/unmapping is done whenever a page is freed. I
    >> don't quite see yet how that would help to remember if a page was
    >> already reported.
    >
    > Do you still have to differ that state when each page is reported?

    Ah, very good point. I can see that the reason for this was not
    discussed in this thread so far. (Alexander, Nitesh, please correct me
    if I am wrong). It's buried in the long history of free page
    hinting/reporting.

    Some early patch sets tried to report during every free synchronously.
    Free a page, report them to the hypervisor. This resulted in some issues
    (especially, locking-related and the virtio + the hypervisor being
    involved, resulting in unpredictable delays, quite some overhead ...).
    It was no good.

    One design decision then was to not report single pages, but a bunch of
    pages at once. This made it necessary to "remember" the pages to be
    reported and to temporarily block them from getting allocated while
    reporting.

    Nitesh implemented (at least) two "capture PFNs of free pages in an
    array when freeing" approaches. One being synchronous from the freeing
    CPU once the list was full (having similar issues as plain synchronous
    reporting) and one being asynchronous by a separate thread (which solved
    many locking issues).

    Turned out the a simple array can quickly lead to us having to drop
    "reports" to the hypervisor because the array is full and the reporting
    thread was not able to keep up. Not good as well. Especially, if some
    process frees a lot of memory this can happen quickly and Nitesh wa
    sable to trigger this scenario frequently.

    Finally, Nitesh decided to use the bitmap instead to keep track of pages
    to report. I'd like to note that this approach could still be combined
    with an "array of potentially free PFNs". Only when the array/circular
    buffer runs out of entries ("reporting thread cannot keep up"), we would
    have to go back to scanning the bitmap.

    That was also the point where Alexander decided to look into integrating
    tracking/handling reported/unreported pages directly in the buddy.

    >
    >> After reporting the page we would have to switch some
    >> state (Nitesh: bitmap bit, Alexander: page flag) to identify that.
    >
    > Yes, you can either store the state somewhere.
    >
    >> Of course, we could map the page and treat that as "the state" when we
    >> reported it, but I am not sure that's such a good idea :)
    >>
    >> As always, I might be very wrong ...
    >
    > I still do not fully understand the usecase so I might be equally wrong.
    > My thinking is along these lines. Why should you scan free pages when
    > you can effectively capture each freed page? If you go one step further
    > then post_alloc_hook would be the counterpart to know that your page has
    > been allocated.

    I'd like to note that Nitesh's patch set contains the following hunk,
    which is roughly what you were thinking :)


    -static inline void __free_one_page(struct page *page,
    +inline void __free_one_page(struct page *page,
    unsigned long pfn,
    struct zone *zone, unsigned int order,
    - int migratetype)
    + int migratetype, bool hint)
    {
    unsigned long combined_pfn;
    unsigned long uninitialized_var(buddy_pfn);
    @@ -980,7 +981,8 @@ static inline void __free_one_page(struct page *page,
    migratetype);
    else
    add_to_free_area(page, &zone->free_area[order], migratetype);
    -
    + if (hint)
    + page_hinting_enqueue(page, order);
    }


    (ignore the hint parameter, when he would switch to a isolate vs.
    alloc/free, that can go away and all we left is the enqueue part)

    Inside that callback we can remember the pages any way we want. Right
    now in a bitmap. Maybe later in a array + bitmap (as discussed above).
    Another idea I had was to simply go over all pages and report them when
    running into this "array full" condition. But I am not yet sure about
    the performance implications on rather large machines. So the bitmap
    idea might have some other limitations but seems to do its job.

    Hoe that makes things clearer and am not missing something.

    --

    Thanks,

    David / dhildenb

    \
     
     \ /
      Last update: 2019-09-12 09:47    [W:2.208 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site