Messages in this thread | | | From | "Huang\, Ying" <> | Subject | Re: [PATCH -mm -v9 1/3] mm, THP, swap: Delay splitting THP during swap out | Date | Thu, 20 Apr 2017 08:43:12 +0800 |
| |
Hi, Johannes,
Johannes Weiner <hannes@cmpxchg.org> writes:
> On Wed, Apr 19, 2017 at 03:06:23PM +0800, Huang, Ying wrote: >> @@ -206,17 +212,34 @@ int add_to_swap(struct page *page, struct list_head *list) >> */ >> err = add_to_swap_cache(page, entry, >> __GFP_HIGH|__GFP_NOMEMALLOC|__GFP_NOWARN); >> - >> - if (!err) { >> - return 1; >> - } else { /* -ENOMEM radix-tree allocation failure */ >> + /* -ENOMEM radix-tree allocation failure */ >> + if (err) >> /* >> * add_to_swap_cache() doesn't return -EEXIST, so we can safely >> * clear SWAP_HAS_CACHE flag. >> */ >> - swapcache_free(entry); >> - return 0; >> + goto fail_free; >> + >> + if (unlikely(PageTransHuge(page))) { >> + err = split_huge_page_to_list(page, list); >> + if (err) { >> + delete_from_swap_cache(page); >> + return 0; >> + } >> } >> + >> + return 1; >> + >> +fail_free: >> + if (unlikely(PageTransHuge(page))) >> + swapcache_free_cluster(entry); >> + else >> + swapcache_free(entry); >> +fail: >> + if (unlikely(PageTransHuge(page)) && >> + !split_huge_page_to_list(page, list)) >> + goto retry; > > May I ask why you added the unlikelies there? Can you generally say > THPs are unlikely in this path? Is the swap-out path so hot that > branch layout is critical? I doubt either is true.
I just found there are unlikely() encloses PageTransHuge() in the original add_to_swap(), so I just follow the original style. But I don't think they make much sense too. Will remove them in the next version.
> Also please mention changes like these in the changelog next time.
Sorry and will do that in the future.
Best Regards, Huang, Ying
| |