Re: [LTP] [PATCH] hugepages: ensure CONFIG_COMPACTION is defined

Hi Andrea,

> From: Andrea Cervesato <[email protected]>

> --- a/lib/tst_hugepage.c
> +++ b/lib/tst_hugepage.c
> @@ -46,6 +46,11 @@ unsigned long tst_reserve_hugepages(struct tst_hugepage *hp)
>  		goto set_hugepages;
>  	}
>  
> +	if (access(PATH_VM_COMPACT_MEMORY, F_OK)) {
> +		if (hp->policy == TST_NEEDS || hp->policy == TST_REQUEST)
> +			tst_brk(TCONF, "CONFIG_COMPACTION not enabled in kernel");
> +	}
> +

Thanks for the patch. I think the goal is reasonable, but I'd
push back on the core assumption: compact_memory is not actually
required to reserve hugepages.

Writing 1 to /proc/sys/vm/compact_memory only triggers proactive
memory compaction. It's a best-effort hint that makes a hugepage
reservation more likely to succeed by defragmenting memory
beforehand, but the reservation itself does not depend on it.

The kernel can (and often will) still satisfy the request without
any explicit compaction step. So treating a missing compact_memory
(i.e. CONFIG_COMPACTION=n) as a hard TCONF is too strict: it skips
tests that could otherwise run perfectly fine.

Given that, I don't think we should tst_brk(TCONF) here at all.
My suggetion is not to use SAFE_* macros:

--- a/lib/tst_hugepage.c
+++ b/lib/tst_hugepage.c
@@ -47,7 +47,7 @@ unsigned long tst_reserve_hugepages(struct tst_hugepage *hp)
        }
 
        SAFE_FILE_PRINTF(PATH_VM_DROP_CACHES, "3");
-       SAFE_FILE_PRINTF(PATH_VM_COMPACT_MEMORY, "1");
+       FILE_PRINTF(PATH_VM_COMPACT_MEMORY, "1");
        if (hp->policy == TST_NEEDS) {
                tst_hugepages += SAFE_READ_MEMINFO("HugePages_Total:");
                goto set_hugepages;


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp