On 28.09.20 09:58, Oscar Salvador wrote: > On Wed, Sep 16, 2020 at 08:34:11PM +0200, David Hildenbrand wrote: >> @@ -1523,7 +1524,13 @@ void __free_pages_core(struct page *page, unsigned int order) >> >> atomic_long_add(nr_pages, &page_zone(page)->managed_pages); >> set_page_refcounted(page); >> - __free_pages(page, order); >> + >> + /* >> + * Bypass PCP and place fresh pages right to the tail, primarily >> + * relevant for memory onlining. >> + */ >> + page_ref_dec(page); >> + __free_pages_ok(page, order, FOP_TO_TAIL); > > Sorry, I must be missing something obvious here, but I am a bit confused here. > I get the part of placing them at the tail so rmqueue_bulk() won't > find them, but I do not get why we decrement page's refcount. > IIUC, its refcount will be 0, but why do we want to do that? > > Another thing a bit unrelated... we mess three times with page's refcount > (two before this patch). > Why do we have this dance in place? Hi Oscar! Old code: set_page_refcounted(): sets the refcount to 1. __free_pages() -> put_page_testzero(): sets it to 0 -> free_the_page()->__free_pages_ok() New code: set_page_refcounted(): sets the refcount to 1. page_ref_dec(page): sets it to 0 __free_pages_ok(): We could skip the set_page_refcounted() + page_ref_dec(page) and lose a couple of sanity checks but we could simply use a VM_BUG_ON_PAGE(page_ref_count(page), page), which is what we really care about when onlining memory. -- Thanks, David / dhildenb
