Re: [LTP] [PATCH V4 5/6] libswap: Introduce file contiguity check

Petr Vorel
2024-01-24T18:17:53Z

Hi Li,

> This patch introduces a new function file_is_contiguous to the
> libltpswap library to determine if a swap file is stored in a
> contiguous block of disk space, which is a typical requirement
> for swap files in Linux. The function performs a series of checks
> using the fiemap structure to assess the contiguity of the file
> and logs the result.

> It is integrated into the is_swap_supported function to replace
> the previous tst_fibmap check, providing a more reliable method
> for verifying that a file suitable for swap is indeed contiguous.

...
> +static int file_is_contiguous(const char *filename)
> +{
> + int fd, contiguous = 0;
> + struct fiemap *fiemap;
> +
> + if (tst_fibmap(filename) == 0) {
> + contiguous = 1;
> + goto out;
> + }
> +
> + if (tst_fs_type(filename) == TST_TMPFS_MAGIC) {
> + contiguous = 0;
nit: contiguous is already 0. Also I like that tst_fibmap is tested first (in
case tmpfs gets that support one day.
> + goto out;
> + }
> +
> + fd = SAFE_OPEN(filename, O_RDONLY);
> +
> + fiemap = (struct fiemap *)SAFE_MALLOC(sizeof(struct fiemap) + sizeof(struct fiemap_extent));
> + memset(fiemap, 0, sizeof(struct fiemap) + sizeof(struct fiemap_extent));
> +
> + fiemap->fm_start = 0;
> + fiemap->fm_length = ~0;
> + fiemap->fm_flags = 0;
> + fiemap->fm_extent_count = 1;
> +
> + SAFE_IOCTL(fd, FS_IOC_FIEMAP, fiemap);
> +
> + /*
> + * fiemap->fm_mapped_extents != 1:
> + * This checks if the file does not have exactly one extent. If there are more
> + * or zero extents, the file is not stored in a single contiguous block.
> + *
> + * fiemap->fm_extents[0].fe_logical != 0:
> + * This checks if the first extent does not start at the logical offset 0 of
> + * the file. If it doesn't, it indicates that the file's first block of data
> + * is not at the beginning of the file, which implies non-contiguity.
> + *
> + * (fiemap->fm_extents[0].fe_flags & FIEMAP_EXTENT_LAST) != FIEMAP_EXTENT_LAST:
> + * This checks if the first extent does not have the FIEMAP_EXTENT_LAST flag set.
> + * If the flag isn't set, it means that this extent is not the last one, suggesting
> + * that there are more extents and the file is not contiguous.
> + */

Interesting, thanks for the doc!

LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr