Re: [PATCH -mm -v9 1/3] mm, THP, swap: Delay splitting THP during swap out

Johannes Weiner
2017-04-19T15:53:06Z

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.

Also please mention changes like these in the changelog next time.