Re: [LTP] [PATCH v6 2/2] ioctl_fiemap01: New test for fiemap ioctl()

Cyril Hrubis
2025-04-11T15:40:49Z

Hi!
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c b/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c
> new file mode 100644
> index 000000000..82f67adcf
> --- /dev/null
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * Verify basic fiemap ioctl functionality, including:
> + *
> + * - The ioctl returns EBADR if it receives invalid fm_flags.
> + * - 0 extents are reported for an empty file.
> + * - The ioctl correctly retrieves single and multiple extent mappings after writing to the file.
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/fiemap.h>
> +#include <stdlib.h>
> +#include <sys/statvfs.h>
> +
> +#include "tst_test.h"
> +
> +#define MNTPOINT "mntpoint"
> +#define TESTFILE "testfile"
> +#define NUM_EXTENT 3
> +
> +static void print_extens(struct fiemap *fiemap)
> +{
> + tst_res(TDEBUG, "File extent count: %u", fiemap->fm_mapped_extents);
> +
> + for (unsigned int i = 0; i < fiemap->fm_mapped_extents; ++i) {
> + tst_res(TDEBUG, "Extent %u: Logical offset: %llu, Physical offset: %llu, flags: %u, Length: %llu",
> + i + 1,
> + fiemap->fm_extents[i].fe_logical,
> + fiemap->fm_extents[i].fe_physical,
> + fiemap->fm_extents[i].fe_flags,
> + fiemap->fm_extents[i].fe_length);
> + }
> +}
> +
> +static void check_extent(struct fiemap *fiemap, unsigned int fm_mapped_extents,
> + int index_extents, int fe_flags,
> + unsigned int min_fe_physical,

There is no point in passing the value of min_fe_physical if we just
pass 1 in all cases, we can just hardcode the value in the function.

> + unsigned int fe_length)
> +{
> + TST_EXP_EXPR(fiemap->fm_mapped_extents == fm_mapped_extents,
> + "Check extent fm_mapped_extents is %d", fm_mapped_extents);

This check should be done in a separate function instead, because we
want to check all three extents in the last test and there is no point
in checking this three times.

Also we do have TST_EXP_EQ_UI() which should be used in the cases we are
testing for equality.

> + TST_EXP_EXPR(fiemap->fm_extents[index_extents].fe_flags & fe_flags,
^
This does not really work if we want to assert if the last flag was set
only on the last.

I.e. if for the last test we do:

check_extent(fiemap, NUM_EXTENT, 0, 0, blk_size);
check_extent(fiemap, NUM_EXTENT, 1, 0, blk_size);
check_extent(fiemap, NUM_EXTENT, 2, FIEMAP_EXTENT_LAST, blk_size);

The fe_flags test fails because we get zero. What we have to do instead
is to pass the mask and the value and check for equality as:

TST_EXP_EQ_UI((fiemap->fm_extents[index_extents].fe_flags & fe_mask) == fe_flags);

And then we call it like:

check_extent(fiemap, NUM_EXTENT, 0, FIEMAP_EXTENT_LAST, 0, blk_size);
check_extent(fiemap, NUM_EXTENT, 1, FIEMAP_EXTENT_LAST, 0, blk_size);
check_extent(fiemap, NUM_EXTENT, 2, FIEMAP_EXTENT_LAST, FIEMAP_EXTENT_LAST, blk_size);

> + "Check fe_flags is %d", fe_flags);
> + TST_EXP_EXPR(fiemap->fm_extents[index_extents].fe_physical >= min_fe_physical,
> + "Check fe_physical > %d", min_fe_physical);
> + TST_EXP_EXPR(fiemap->fm_extents[index_extents].fe_length == fe_length,
^
And lastly but not least it would be shorter to
get the pointer to the extent and the start as:

struct fiemap_extent *extent = &fieamp->fm_extents[index_extents];

And then do just extent->fe_foo in all cases.

> + "Check fe_length is %d", fe_length);
> +}
> +
> +static void verify_ioctl(void)
> +{
> + int fd;
> + struct fiemap *fiemap;
> + struct statvfs fs_info;
> + unsigned long blk_size;
> +
> + fd = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, 0644);
> +
> + SAFE_STATVFS(".", &fs_info);
> +
> + blk_size = fs_info.f_bsize;
> +
> + fiemap = SAFE_MALLOC(sizeof(struct fiemap) + sizeof(struct fiemap_extent) * NUM_EXTENT);
> + fiemap->fm_start = 0;
> + fiemap->fm_length = ~0ULL;
> + fiemap->fm_extent_count = 1;
> +
> + fiemap->fm_flags = -1;
> + TST_EXP_FAIL(ioctl(fd, FS_IOC_FIEMAP, fiemap), EBADR);
> +
> + fiemap->fm_flags = 0;
> + TST_EXP_PASS(ioctl(fd, FS_IOC_FIEMAP, fiemap));
> + print_extens(fiemap);
> + TST_EXP_EXPR(fiemap->fm_mapped_extents == 0,
> + "Empty file should have 0 extends mapped");
^
extents
> +
> + char *buf = SAFE_MALLOC(blk_size);
> +
> + SAFE_WRITE(SAFE_WRITE_ANY, fd, buf, blk_size);
> + fiemap->fm_flags = FIEMAP_FLAG_SYNC;
> + TST_EXP_PASS(ioctl(fd, FS_IOC_FIEMAP, fiemap));
> + print_extens(fiemap);
> + check_extent(fiemap, 1, 0, FIEMAP_EXTENT_LAST, 1, blk_size);
> +
> + fiemap->fm_extent_count = NUM_EXTENT;
> + SAFE_LSEEK(fd, 2 * blk_size, SEEK_SET);
> + SAFE_WRITE(SAFE_WRITE_ALL, fd, buf, blk_size);
> + SAFE_LSEEK(fd, 4 * blk_size, SEEK_SET);
> + SAFE_WRITE(SAFE_WRITE_ALL, fd, buf, blk_size);
> + TST_EXP_PASS(ioctl(fd, FS_IOC_FIEMAP, fiemap));
> + print_extens(fiemap);
> + check_extent(fiemap, NUM_EXTENT, NUM_EXTENT - 1, FIEMAP_EXTENT_LAST, 1, blk_size);
> +
> + free(buf);
> + free(fiemap);
> + SAFE_CLOSE(fd);
> + SAFE_UNLINK(TESTFILE);
> +}
> +
> +static void setup(void)
> +{
> + SAFE_CHDIR(MNTPOINT);
> +}
> +
> +static struct tst_test test = {
> + .setup = setup,
> + .mount_device = 1,
> + .mntpoint = MNTPOINT,
> + .all_filesystems = 1,
> + .skip_filesystems = (const char *const[]) {
> + "exfat", "vfat", "fuse", "ntfs", "tmpfs", NULL
> + },
> + .test_all = verify_ioctl,
> + .needs_root = 1,
> +};
> --
> 2.35.3
>

--
Cyril Hrubis
chrubis@suse.cz