On 05/06/2015 12:38 PM, Andrew Morton wrote:
On Mon, 04 May 2015 16:14:48 -0700 Alexander Duyck <alexander.h.duyck@xxxxxxxxxx> wrote:+/** + * skb_free_frag - free a page fragment + * @head: virtual address of page fragment + * + * Frees a page fragment allocated out of either a compound or order 0 page. + * The function itself is a hybrid between free_pages and free_compound_page + * which can be found in mm/page_alloc.c + */ +void skb_free_frag(void *head) +{ + struct page *page = virt_to_head_page(head); + + if (unlikely(put_page_testzero(page))) { + if (likely(PageHead(page))) + __free_pages_ok(page, compound_order(page)); + else + free_hot_cold_page(page, false); + } +}Why are we testing for PageHead in here? If the code were to simply do if (unlikely(put_page_testzero(page))) __free_pages_ok(page, compound_order(page)); that would still work?
My assumption was that there was a performance difference between __free_pages_ok and free_hot_cold_page for order 0 pages. From what I can tell free_hot_cold_page will do bulk cleanup via free_pcppages_bulk while __free_pages_ok just calls free_one_page.
There's nothing networking-specific in here. I suggest the function be renamed and moved to page_alloc.c. Add an inlined skb_free_frag() in a net header which calls it. This way the mm developers know about it and will hopefully maintain it. It would need a comment explaining when and why people should and shouldn't use it.
That's true. While I am at it I should probably pull the allocation out as well just so it is all in one location.
The term "page fragment" is a net thing and isn't something we know about. What is it? From context I'm thinking a definition would look something like An arbitrary-length arbitrary-offset area of memory which resides within a 0 or higher order page. Multiple fragments within that page are individually refcounted, in the page's reference counter. Is that correct and complete?
Yeah that is pretty complete. I've added Eric who originally authored this to the Cc in case there is something he wants to add. I'll see about updating this and will likely have a v2 ready in the next couple of hours.
- Alex -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>