Re: [PATCH] btrfs: add test for encoded reads

[Mark Harmstone <maharmstone@meta.com>]

On 12/11/24 09:37, Johannes Thumshirn wrote:
> > 
> On 11.11.24 15:56, Mark Harmstone wrote:
>> Add btrfs/333 and its helper programs btrfs_encoded_read and
>> btrfs_encoded_write, in order to test encoded reads.
>>
>> We use the BTRFS_IOC_ENCODED_WRITE ioctl to write random data into a
>> compressed extent, then use the BTRFS_IOC_ENCODED_READ ioctl to check
>> that it matches what we've written. If the new io_uring interface for
>> encoded reads is supported, we also check that that matches the ioctl.
>>
>> Note that what we write isn't valid compressed data, so any non-encoded
>> reads on these files will fail.
>>
>> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
>> ---
>>    .gitignore                |   2 +
>>    src/Makefile              |   3 +-
>>    src/btrfs_encoded_read.c  | 175 ++++++++++++++++++++++++++++++
>>    src/btrfs_encoded_write.c | 206 +++++++++++++++++++++++++++++++++++
>>    tests/btrfs/333           | 220 ++++++++++++++++++++++++++++++++++++++
>>    tests/btrfs/333.out       |   2 +
>>    6 files changed, 607 insertions(+), 1 deletion(-)
>>    create mode 100644 src/btrfs_encoded_read.c
>>    create mode 100644 src/btrfs_encoded_write.c
>>    create mode 100755 tests/btrfs/333
>>    create mode 100644 tests/btrfs/333.out
>>
>> diff --git a/.gitignore b/.gitignore
>> index f16173d9..efd47773 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -62,6 +62,8 @@ tags
>>    /src/attr_replace_test
>>    /src/attr-list-by-handle-cursor-test
>>    /src/bstat
>> +/src/btrfs_encoded_read
>> +/src/btrfs_encoded_write
>>    /src/bulkstat_null_ocount
>>    /src/bulkstat_unlink_test
>>    /src/bulkstat_unlink_test_modified
>> diff --git a/src/Makefile b/src/Makefile
>> index a0396332..b42b8147 100644
>> --- a/src/Makefile
>> +++ b/src/Makefile
>> @@ -34,7 +34,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
>>    	attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \
>>    	fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
>>    	detached_mounts_propagation ext4_resize t_readdir_3 splice2pipe \
>> -	uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault min_dio_alignment
>> +	uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault min_dio_alignment \
>> +	btrfs_encoded_read btrfs_encoded_write
>>    
>>    EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
>>    	      btrfs_crc32c_forged_name.py popdir.pl popattr.py \
>> diff --git a/src/btrfs_encoded_read.c b/src/btrfs_encoded_read.c
>> new file mode 100644
>> index 00000000..a5082f70
>> --- /dev/null
>> +++ b/src/btrfs_encoded_read.c
>> @@ -0,0 +1,175 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (c) Meta Platforms, Inc. and affiliates.
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <sys/uio.h>
>> +#include <sys/ioctl.h>
>> +#include <linux/btrfs.h>
> 
> For this I need
> 
> +#include <linux/io_uring.h>
> 
> otherwise I get:
> 
>       [CC]    btrfs_encoded_read
> /bin/sh ../libtool --quiet --tag=CC --mode=link /usr/bin/gcc-13
> btrfs_encoded_read.c -o btrfs_encoded_read -g -O2 -g -O2 -DDEBUG
> -I../include -DVERSION=\"1.1.1\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
> -funsigned-char -fno-strict-aliasing -Wall -DHAVE_FALLOCATE
> -DNEED_INTERNAL_XFS_IOC_EXCHANGE_RANGE   -lhandle -lacl -lpthread -lrt
> -luuid -lgdbm_compat -lgdbm -laio
>    -luring   ../lib/libtest.la
> btrfs_encoded_read.c: In function 'encoded_read_io_uring':
> btrfs_encoded_read.c:100:26: error: 'IORING_OP_URING_CMD' undeclared
> (first use in this function); did you mean 'IORING_OP_LINKAT'?
>     100 |         io_uring_prep_rw(IORING_OP_URING_CMD, sqe, fd, &enc,
> sizeof(enc), 0);
>         |                          ^~~~~~~~~~~~~~~~~~~
>         |                          IORING_OP_LINKAT
> btrfs_encoded_read.c:100:26: note: each undeclared identifier is
> reported only once for each function it appears in
> btrfs_encoded_read.c:101:12: error: 'struct io_uring_sqe' has no member
> named 'cmd_op'
>     101 |         sqe->cmd_op = BTRFS_IOC_ENCODED_READ;
>         |            ^~
> 
> during compilation.
> 
> Not sure if a ./configure macro thingy should/would solve this.

We could do that, but elsewhere we're using liburing.h rather than the 
kernel version.

It looks like IORING_OP_URING_CMD was added to liburing with version 
2.2, which came out in June 2022. I don't know whether that's old enough 
that we can just declare it as our minimum version, whether we should be 
probing for the liburing version, whether we should be working round 
this somehow, or what.

Zorro, what do you think?

Mark