Re: [LTP] [PATCH 1/3] shell/lib: refactor checkpoint with shared path for exec() support

[Avinesh Kumar <akumar@suse.de>]

Hi Li,

Thank you for this refactoring and fixing the pec test regression,
I tested this patchset.

Tested-by: Avinesh Kumar <akumar@suse.de>


Regards,
Avinesh

On Monday, June 16, 2025 12:26:17 PM CEST Li Wang via ltp wrote:
> This patch refactors shell checkpoint mechanism to support exec()
> based process synchronization by introducing support for a shared
> futex region via a configurable path.
> 
> Key changes:
> 
>  - Introduce LTP_IPC_PATH environment variable to specify the futex
>    shared memory file used for checkpoint synchronization.
>  - Add magic header "LTPM" to validate the shared memory file content.
>  - Add checkpoint_reinit() to re-attach futex mapping for wait/wake
>    operations in exec()'ed processes.
>  - Update tst_checkpoint CLI tool to support "init", "wait", and "wake"
>    commands with a unified interface.
> 
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  include/tst_checkpoint_fn.h    | 20 +++++++++-
>  lib/tst_checkpoint.c           | 70 +++++++++++++++++++++++-----------
>  testcases/lib/tst_checkpoint.c | 27 +++++++------
>  testcases/lib/tst_test.sh      |  2 +
>  4 files changed, 83 insertions(+), 36 deletions(-)
> 
> diff --git a/include/tst_checkpoint_fn.h b/include/tst_checkpoint_fn.h
> index 3a010d616..209296fe0 100644
> --- a/include/tst_checkpoint_fn.h
> +++ b/include/tst_checkpoint_fn.h
> @@ -6,13 +6,29 @@
>  #define TST_CHECKPOINT_FN__
>  
>  /*
> - * Checkpoint initializaton, must be done first.
> + * Checkpoint initialization.
>   *
> - * NOTE: tst_tmpdir() must be called beforehand.
> + * This function sets up the shared memory region used for process
> + * synchronization via futexes. It must be called before any checkpoint
> + * operations such as tst_checkpoint_wait() or tst_checkpoint_wake().
>   */
>  void tst_checkpoint_init(const char *file, const int lineno,
>  			 void (*cleanup_fn)(void));
>  
> +/*
> + * Checkpoint reinitialization.
> + *
> + * This function re-attaches to an existing shared memory checkpoint region
> + * pointed to by the LTP_IPC_PATH environment variable. It is typically used
> + * in child processes (e.g., shell scripts) to synchronize with the main test.
> + *
> + * The function verifies the magic header in the shared memory file and maps
> + * the futex array into memory. It must be called before using checkpoint
> + * operations in a process that did not perform the original initialization.
> + */
> +void tst_checkpoint_reinit(const char *file, const int lineno,
> +			   void (*cleanup_fn)(void));
> +
>  /*
>   * Waits for wakeup.
>   *
> diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
> index 6a294b28b..5efbf98d2 100644
> --- a/lib/tst_checkpoint.c
> +++ b/lib/tst_checkpoint.c
> @@ -38,8 +38,9 @@ unsigned int tst_max_futexes;
>  void tst_checkpoint_init(const char *file, const int lineno,
>                           void (*cleanup_fn)(void))
>  {
> +	char *path = getenv("LTP_IPC_PATH");
> +	size_t page_size = getpagesize();
>  	int fd;
> -	unsigned int page_size;
>  
>  	if (tst_futexes) {
>  		tst_brkm_(file, lineno, TBROK, cleanup_fn,
> @@ -47,35 +48,58 @@ void tst_checkpoint_init(const char *file, const int lineno,
>  		return;
>  	}
>  
> -	/*
> -	 * The parent test process is responsible for creating the temporary
> -	 * directory and therefore must pass non-zero cleanup (to remove the
> -	 * directory if something went wrong).
> -	 *
> -	 * We cannot do this check unconditionally because if we need to init
> -	 * the checkpoint from a binary that was started by exec() the
> -	 * tst_tmpdir_created() will return false because the tmpdir was
> -	 * created by parent. In this case we expect the subprogram can call
> -	 * the init as a first function with NULL as cleanup function.
> -	 */
> -	if (cleanup_fn && !tst_tmpdir_created()) {
> -		tst_brkm_(file, lineno, TBROK, cleanup_fn,
> -			"You have to create test temporary directory "
> -			"first (call tst_tmpdir())");
> -		return;
> +	if (!path) {
> +		char *tmp_path = NULL;
> +
> +		if (!tst_tmpdir_created())
> +			tst_tmpdir();
> +
> +		safe_asprintf(__FILE__, __LINE__, cleanup_fn, &tmp_path,
> +				"%s/ltp_checkpoint", tst_get_tmpdir());
> +		path = tmp_path;
>  	}
>  
> -	page_size = getpagesize();
> +	fd = SAFE_OPEN(cleanup_fn, path, O_RDWR | O_CREAT, 0666);
> +	SAFE_WRITE(cleanup_fn, 1, fd, "LTPM", 4);
> +
> +	tst_futexes = SAFE_MMAP(cleanup_fn, NULL, page_size,
> +				PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> +
> +	tst_futexes = (futex_t *)((char *)tst_futexes + 4);
> +	tst_max_futexes = (page_size - 4) / sizeof(futex_t);
> +
> +	SAFE_CLOSE(cleanup_fn, fd);
> +}
> +
> +void tst_checkpoint_reinit(const char *file, const int lineno,
> +			   void (*cleanup_fn)(void))
> +{
> +	const char *path = getenv("LTP_IPC_PATH");
> +	size_t page_size = getpagesize();
> +	int fd;
>  
> -	fd = SAFE_OPEN(cleanup_fn, "checkpoint_futex_base_file",
> -	               O_RDWR | O_CREAT, 0666);
> +	if (!path) {
> +		tst_brkm_(file, lineno, TBROK, cleanup_fn,
> +				"LTP_IPC_PATH is not defined");
> +	}
>  
> -	SAFE_FTRUNCATE(cleanup_fn, fd, page_size);
> +	if (access(path, F_OK)) {
> +		tst_brkm_(file, lineno, TBROK, cleanup_fn,
> +				"File %s does not exist!", path);
> +	}
>  
> +	fd = SAFE_OPEN(cleanup_fn, path, O_RDWR);
>  	tst_futexes = SAFE_MMAP(cleanup_fn, NULL, page_size,
> -	                    PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> +			PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> +
> +	char *ptr = (char *)tst_futexes;
> +	if (memcmp(ptr, "LTPM", 4) != 0) {
> +		tst_brkm_(file, lineno, TBROK, cleanup_fn,
> +				"Invalid shared memory region (bad magic)");
> +	}
>  
> -	tst_max_futexes = page_size / sizeof(uint32_t);
> +	tst_futexes = (futex_t *)((char *)tst_futexes + 4);
> +	tst_max_futexes = (page_size - 4) / sizeof(futex_t);
>  
>  	SAFE_CLOSE(cleanup_fn, fd);
>  }
> diff --git a/testcases/lib/tst_checkpoint.c b/testcases/lib/tst_checkpoint.c
> index c70c4e8e5..35a0c0dfa 100644
> --- a/testcases/lib/tst_checkpoint.c
> +++ b/testcases/lib/tst_checkpoint.c
> @@ -9,11 +9,14 @@
>  #include <stdlib.h>
>  #define TST_NO_DEFAULT_MAIN
>  #include "tst_test.h"
> +#include "tst_checkpoint.h"
>  
>  static void print_help(void)
>  {
> -	printf("Usage: tst_checkpoint wait TIMEOUT ID\n");
> +	printf("Usage: tst_checkpoint init\n");
> +	printf("   or: tst_checkpoint wait TIMEOUT ID\n");
>  	printf("   or: tst_checkpoint wake TIMEOUT ID NR_WAKE\n");
> +	printf("Arguments:\n");
>  	printf("       TIMEOUT - timeout in ms\n");
>  	printf("       ID - checkpoint id\n");
>  	printf("       NR_WAKE - number of processes to wake up\n");
> @@ -43,8 +46,13 @@ int main(int argc, char *argv[])
>  	int ret;
>  	int type;
>  
> -	if (argc < 3)
> -		goto help;
> +	if (!strcmp(argv[1], "init")) {
> +		if (argc != 2)
> +			goto help;
> +
> +		tst_checkpoint_init(__FILE__, __LINE__, NULL);
> +		return 0;
> +	}
>  
>  	if (!strcmp(argv[1], "wait")) {
>  		type = 0;
> @@ -70,17 +78,14 @@ int main(int argc, char *argv[])
>  		goto help;
>  	}
>  
> -	tst_reinit();
> +	tst_checkpoint_reinit(__FILE__, __LINE__, NULL);
>  
> -	if (type)
> -		ret = tst_checkpoint_wake(id, nr_wake, timeout);
> -	else
> +	if (type == 0)
>  		ret = tst_checkpoint_wait(id, timeout);
> -
> -	if (ret)
> -		return 1;
>  	else
> -		return 0;
> +		ret = tst_checkpoint_wake(id, nr_wake, timeout);
> +
> +	return ret ? 1 : 0;
>  
>  help:
>  	print_help();
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index c32bd8b19..e5e76067b 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -629,6 +629,8 @@ _tst_init_checkpoints()
>  	ROD_SILENT dd if=/dev/zero of="$LTP_IPC_PATH" bs="$pagesize" count=1
>  	ROD_SILENT chmod 600 "$LTP_IPC_PATH"
>  	export LTP_IPC_PATH
> +
> +	tst_checkpoint init
>  }
>  
>  _prepare_device()
> 





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