Re: [PATCH v2 1/9] mm: Separate free page chunk merging into its own routine

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

>>>> +        }
>>>> +
>>>> +        order++;
>>>> +    }
>>>> +
>>>> +    PFN_ORDER(pg) = order;
>>>> +    page_list_add_tail(pg, &heap(node, zone, order));
>>> I don't think this strictly is part of the merge anymore, so judging
>>> by the name of the function this last line would rather belong into
>>> the caller.
>> All users of merge_chunks() want to place the new chunk back into the
>> heap and I don't think I see why anyone would want to just get the new
>> buddy without putting it there.
> I understand that, yet the name of the function says otherwise.
> At the very least I'd expect a comment right ahead of the function
> to state this. Or maybe, as a slight variation of the new name you
> suggest above, free_merged_chunks() (merge_chunk_sand_free()
> or some such would look ugly to me)?

merge_and_free_chunks(). And, in fact, merge_and_free_buddy(), and
probably s/chunk/buddy/ in scrub_free_pages(), introduced in a later patch.

-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel