Re: [PATCH 2/2] fsx: add missing fallocate flag ifdefs

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Sep 26, 2024 at 10:41:47AM -0400, Brian Foster wrote:
> The various fallocate flags are mostly ifdef'd for backward
> compatibility with the exception of the associated test_fallocate()
> calls to verify functionality at runtime. I suspect the reason for
> this was to avoid ifdef ugliness around having to clear the runtime
> flag for each operation, but unfortunately this defeats the purpose
> of the ifdef protection everywhere else.
> 
> Factor out the fallocate related test calls into a new helper and
> add the appropriate ifdefs.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  ltp/fsx.c | 59 ++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 45 insertions(+), 14 deletions(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 677f8c9f..417743c5 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -2833,6 +2833,50 @@ __test_fallocate(int mode, const char *mode_str)
>  #endif
>  }
>  
> +void
> +test_fallocate_calls(void)
> +{
> +	if (fallocate_calls)
> +		fallocate_calls = test_fallocate(0);
> +	if (keep_size_calls)
> +		keep_size_calls = test_fallocate(FALLOC_FL_KEEP_SIZE);
> +
> +#ifdef FALLOC_FL_UNSHARE_RANGE
> +	if (unshare_range_calls)
> +		unshare_range_calls = test_fallocate(FALLOC_FL_UNSHARE_RANGE);
> +#else
> +	unshare_range_calls = 0;
> +#endif
> +
> +#ifdef FALLOC_FL_PUNCH_HOLE
> +	if (punch_hole_calls)
> +		punch_hole_calls = test_fallocate(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE);
> +#else
> +	punch_hole_calls = 0;
> +#endif
> +
> +#ifdef FALLOC_FL_ZERO_RANGE
> +	if (zero_range_calls)
> +		zero_range_calls = test_fallocate(FALLOC_FL_ZERO_RANGE);
> +#else
> +	zero_range_calls = 0;
> +#endif
> +
> +#ifdef FALLOC_FL_COLLAPSE_RANGE
> +	if (collapse_range_calls)
> +		collapse_range_calls = test_fallocate(FALLOC_FL_COLLAPSE_RANGE);
> +#else
> +	collapse_range_calls = 0;
> +#endif

The concept looks fine, but collapse and zero range have been in the
kernel for a decade now, do we really need to have ifdef tests for them?

--D

> +
> +#ifdef FALLOC_FL_INSERT_RANGE
> +	if (insert_range_calls)
> +		insert_range_calls = test_fallocate(FALLOC_FL_INSERT_RANGE);
> +#else
> +	insert_range_calls = 0;
> +#endif
> +}
> +
>  bool
>  keep_running(void)
>  {
> @@ -3271,20 +3315,7 @@ main(int argc, char **argv)
>  		check_trunc_hack();
>  	}
>  
> -	if (fallocate_calls)
> -		fallocate_calls = test_fallocate(0);
> -	if (keep_size_calls)
> -		keep_size_calls = test_fallocate(FALLOC_FL_KEEP_SIZE);
> -	if (unshare_range_calls)
> -		unshare_range_calls = test_fallocate(FALLOC_FL_UNSHARE_RANGE);
> -	if (punch_hole_calls)
> -		punch_hole_calls = test_fallocate(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE);
> -	if (zero_range_calls)
> -		zero_range_calls = test_fallocate(FALLOC_FL_ZERO_RANGE);
> -	if (collapse_range_calls)
> -		collapse_range_calls = test_fallocate(FALLOC_FL_COLLAPSE_RANGE);
> -	if (insert_range_calls)
> -		insert_range_calls = test_fallocate(FALLOC_FL_INSERT_RANGE);
> +	test_fallocate_calls();
>  	if (clone_range_calls)
>  		clone_range_calls = test_clone_range();
>  	if (dedupe_range_calls)
> -- 
> 2.46.1
> 
>