[PATCH v2] mm: remove PageTransCompound()

[PATCH v2] mm: remove PageTransCompound()

[Kefeng Wang]

Remove the last user of PageTransCompound() in ksm and
get rid of PageTransCompound().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
v2:
- use more folio in cmp_and_merge_page(), suggested by David

 include/linux/page-flags.h | 14 --------------
 mm/ksm.c                   | 17 +++++++++--------
 2 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 7223f6f4e2b4..7a863572adce 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -879,20 +879,6 @@ FOLIO_FLAG_FALSE(partially_mapped)
 
 #define PG_head_mask ((1UL << PG_head))
 
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-/*
- * PageTransCompound returns true for both transparent huge pages
- * and hugetlbfs pages, so it should only be called when it's known
- * that hugetlbfs pages aren't involved.
- */
-static inline int PageTransCompound(const struct page *page)
-{
-	return PageCompound(page);
-}
-#else
-TESTPAGEFLAG_FALSE(TransCompound, transcompound)
-#endif
-
 #if defined(CONFIG_MEMORY_FAILURE) && defined(CONFIG_TRANSPARENT_HUGEPAGE)
 /*
  * PageHasHWPoisoned indicates that at least one subpage is hwpoisoned in the
diff --git a/mm/ksm.c b/mm/ksm.c
index 7d5b76478f0b..41ab25aa2a82 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -2327,23 +2327,24 @@ static void cmp_and_merge_page(struct page *page, struct ksm_rmap_item *rmap_ite
 	tree_rmap_item =
 		unstable_tree_search_insert(rmap_item, page, &tree_page);
 	if (tree_rmap_item) {
+		struct folio *tree_folio;
 		bool split;
 
 		kfolio = try_to_merge_two_pages(rmap_item, page,
 						tree_rmap_item, tree_page);
+		tree_folio = page_folio(tree_page);
 		/*
-		 * If both pages we tried to merge belong to the same compound
-		 * page, then we actually ended up increasing the reference
-		 * count of the same compound page twice, and split_huge_page
-		 * failed.
+		 * If both pages we tried to merge belong to the same (large)
+		 * folio, then we actually ended up increasing the reference
+		 * count of the same folio twice, and split_huge_page failed.
+		 *
 		 * Here we set a flag if that happened, and we use it later to
-		 * try split_huge_page again. Since we call put_page right
+		 * try split_huge_page again. Since we call folio_put() right
 		 * afterwards, the reference count will be correct and
 		 * split_huge_page should succeed.
 		 */
-		split = PageTransCompound(page)
-			&& compound_head(page) == compound_head(tree_page);
-		put_page(tree_page);
+		split = folio == tree_folio;
+		folio_put(tree_folio);
 		if (kfolio) {
 			/*
 			 * The pages were successfully merged: insert new
-- 
2.27.0

Re: [PATCH v2] mm: remove PageTransCompound()

[David Hildenbrand (Arm)]

On 6/18/26 13:35, Kefeng Wang wrote:
> Remove the last user of PageTransCompound() in ksm and
> get rid of PageTransCompound().
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> v2:
> - use more folio in cmp_and_merge_page(), suggested by David
> 
>  include/linux/page-flags.h | 14 --------------
>  mm/ksm.c                   | 17 +++++++++--------
>  2 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 7223f6f4e2b4..7a863572adce 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -879,20 +879,6 @@ FOLIO_FLAG_FALSE(partially_mapped)
>  
>  #define PG_head_mask ((1UL << PG_head))
>  
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -/*
> - * PageTransCompound returns true for both transparent huge pages
> - * and hugetlbfs pages, so it should only be called when it's known
> - * that hugetlbfs pages aren't involved.
> - */
> -static inline int PageTransCompound(const struct page *page)
> -{
> -	return PageCompound(page);
> -}
> -#else
> -TESTPAGEFLAG_FALSE(TransCompound, transcompound)
> -#endif
> -
>  #if defined(CONFIG_MEMORY_FAILURE) && defined(CONFIG_TRANSPARENT_HUGEPAGE)
>  /*
>   * PageHasHWPoisoned indicates that at least one subpage is hwpoisoned in the
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 7d5b76478f0b..41ab25aa2a82 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -2327,23 +2327,24 @@ static void cmp_and_merge_page(struct page *page, struct ksm_rmap_item *rmap_ite
>  	tree_rmap_item =
>  		unstable_tree_search_insert(rmap_item, page, &tree_page);
>  	if (tree_rmap_item) {
> +		struct folio *tree_folio;
>  		bool split;
>  
>  		kfolio = try_to_merge_two_pages(rmap_item, page,
>  						tree_rmap_item, tree_page);
> +		tree_folio = page_folio(tree_page);
>  		/*
> -		 * If both pages we tried to merge belong to the same compound
> -		 * page, then we actually ended up increasing the reference
> -		 * count of the same compound page twice, and split_huge_page
> -		 * failed.
> +		 * If both pages we tried to merge belong to the same (large)
> +		 * folio, then we actually ended up increasing the reference
> +		 * count of the same folio twice, and split_huge_page failed.
> +		 *
>  		 * Here we set a flag if that happened, and we use it later to
> -		 * try split_huge_page again. Since we call put_page right
> +		 * try split_huge_page again. Since we call folio_put() right
>  		 * afterwards, the reference count will be correct and
>  		 * split_huge_page should succeed.
>  		 */
> -		split = PageTransCompound(page)
> -			&& compound_head(page) == compound_head(tree_page);
> -		put_page(tree_page);
> +		split = folio == tree_folio;
> +		folio_put(tree_folio);
>  		if (kfolio) {
>  			/*
>  			 * The pages were successfully merged: insert new

Thanks!

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2] mm: remove PageTransCompound()

[xu.xin16]

> diff --git a/mm/ksm.c b/mm/ksm.c
> index 7d5b76478f0b..41ab25aa2a82 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -2327,23 +2327,24 @@ static void cmp_and_merge_page(struct page *page, struct ksm_rmap_item *rmap_ite
>      tree_rmap_item =
>          unstable_tree_search_insert(rmap_item, page, &tree_page);
>      if (tree_rmap_item) {
> +        struct folio *tree_folio;
>          bool split;
>  
>          kfolio = try_to_merge_two_pages(rmap_item, page,
>                          tree_rmap_item, tree_page);
> +        tree_folio = page_folio(tree_page);
>          /*
> -         * If both pages we tried to merge belong to the same compound
> -         * page, then we actually ended up increasing the reference
> -         * count of the same compound page twice, and split_huge_page
> -         * failed.
> +         * If both pages we tried to merge belong to the same (large)
> +         * folio, then we actually ended up increasing the reference
> +         * count of the same folio twice, and split_huge_page failed.
> +         *
>           * Here we set a flag if that happened, and we use it later to
> -         * try split_huge_page again. Since we call put_page right
> +         * try split_huge_page again. Since we call folio_put() right
>           * afterwards, the reference count will be correct and
>           * split_huge_page should succeed.
>           */
> -        split = PageTransCompound(page)
> -            && compound_head(page) == compound_head(tree_page);
> -        put_page(tree_page);
> +        split = folio == tree_folio;
> +        folio_put(tree_folio);
>          if (kfolio) {
>              /*
>               * The pages were successfully merged: insert new
> -- 
> 2.27.0

LGTM.

Reviewed-by: Xu Xin <xu.xin16@zte.com.cn>
Tested-by: Xu Xin <xu.xin16@zte.com.cn>

and I test it with a reproducer about commit 77da2ba064 ("mm/ksm: fix interaction with THP")

and Test result shows OK.

Happy to share my testing program:
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <ctype.h>

#define THP_SIZE        (1UL << 21)     /* 2MB */
#define WAIT_SECONDS    10              /* time for KSM scans */

/*
 * Read a single integer from a sysfs file.
 */
static int read_sysfs_int(const char *path)
{
        FILE *fp;
        int val;

        fp = fopen(path, "r");
        if (!fp)
                return -1;
        if (fscanf(fp, "%d", &val) != 1) {
                fclose(fp);
                return -1;
        }
        fclose(fp);
        return val;
}

/*
 * Print KSM global counters.
 */
static void print_ksm_counters(int need_verify)
{
        int pages_shared, pages_sharing;

        pages_shared = read_sysfs_int("/sys/kernel/mm/ksm/pages_shared");
        pages_sharing = read_sysfs_int("/sys/kernel/mm/ksm/pages_sharing");
        if (pages_shared >= 0 && pages_sharing >= 0) {
                printf("KSM counters: pages_shared = %d, pages_sharing = %d\n",
                       pages_shared, pages_sharing);
        } else {
                printf("Failed to read KSM counters\n");
        }

        if (need_verify) {
                if (pages_sharing != 510)
                        printf("\n Test result: Failed\n");
                else
                        printf("\n Test result: Passed\n");
        }
}

/*
 * Print relevant fields from /proc/self/smaps for the VMA covering @addr.
 */
static void print_smaps_info(unsigned long addr)
{
        FILE *fp;
        char line[256];
        unsigned long start, end;
        int found = 0;
        int AnonHugePages_val = 0;

        fp = fopen("/proc/self/smaps", "r");
        if (!fp) {
                perror("fopen /proc/self/smaps");
                return;
        }

        while (fgets(line, sizeof(line), fp)) {
                if (sscanf(line, "%lx-%lx", &start, &end) == 2) {
                        if (addr >= start && addr < end) {
                                found = 1;
                                printf("\n[VMA] 0x%lx-0x%lx\n", start, end);
                                /* Read subsequent lines until next VMA */
                                while (fgets(line, sizeof(line), fp)) {
                                        unsigned long dummy_start, dummy_end;
                                        if (sscanf(line, "%lx-%lx",
                                                   &dummy_start, &dummy_end) == 2)
                                                break; /* next VMA */
                                        if (strstr(line, "Rss:") ||
                                            strstr(line, "AnonHugePages:") ||
                                            strstr(line, "Shared_Clean:") ||
                                            strstr(line, "Shared_Dirty:") ||
                                            strstr(line, "Private_Clean:") ||
                                            strstr(line, "Private_Dirty:") ||
                                            strstr(line, "MMUPageSize:"))
                                                printf("%s", line);
                                        if (strstr(line, "AnonHugePages:")) {
                                                if (sscanf(line, "AnonHugePages:%d kB", &AnonHugePages_val) != 1) {
                                                        printf("error: read AnonHugePages\n");
                                                        exit(1);
                                                }
                                        }

                                }
                                break;
                        }
                }
        }

        if (!found) {
                printf("Address 0x%lx not found in smaps\n", addr);
                exit(1);
        }
        if (AnonHugePages_val != 2048) {
                printf("AnonHugePages is not 2048 kb, not Huge page\n");
        }
        fclose(fp);
}

static int thp_is_enabled(void)
{
        FILE *fp;
        char buf[256];
        int ret = 0;

        fp = fopen("/sys/kernel/mm/transparent_hugepage/enabled", "r");
        if (!fp)
                return 0;
        if (fgets(buf, sizeof(buf), fp)) {
                if (strstr(buf, "[always]") || strstr(buf, "[madvise]"))
                        ret = 1;
        }
        fclose(fp);
        return ret;
}

int main(void)
{
        void *p;
        int ret;

        if (!thp_is_enabled()) {
                fprintf(stderr, "Warning: THP seems disabled.\n");
                fprintf(stderr, "Enable with 'echo madvise > /sys/kernel/mm/transparent_hugepage/enabled'\n");
        }

        if (read_sysfs_int("/sys/kernel/mm/ksm/run") != 1) {
                fprintf(stderr, "KSM is not running. Start with 'echo 1 > /sys/kernel/mm/ksm/run'\n");
                return 1;
        }

        ret = posix_memalign(&p, THP_SIZE, THP_SIZE);
        if (ret) {
                fprintf(stderr, "posix_memalign failed: %s\n", strerror(ret));
                return 1;
        }

        if (madvise(p, THP_SIZE, MADV_HUGEPAGE) != 0)
                perror("madvise MADV_HUGEPAGE");
        printf("Allocated %zu bytes at %p (aligned to %zu) with madvise(MADV_HUGEPAGE)\n",
               THP_SIZE, p, THP_SIZE);

        memset(p, 0x42, THP_SIZE);
        printf("Filled with 0x42\n");

        printf("\n=== Before madvise(MADV_MERGEABLE) ===\n");
        print_smaps_info((unsigned long)p);
        print_ksm_counters(0);

        if (madvise(p, THP_SIZE, MADV_MERGEABLE) != 0) {
                perror("madvise MADV_MERGEABLE");
                free(p);
                return 1;
        }
        printf("madvise(MADV_MERGEABLE) succeeded.\n");

        printf("Waiting %d seconds for KSM to scan...\n", WAIT_SECONDS);
        sleep(WAIT_SECONDS);

        printf("\n=== After waiting for KSM ===\n");
        print_smaps_info((unsigned long)p);
        print_ksm_counters(1);

        free(p);
        return 0;
}