"Darrick J. Wong" <djwong@xxxxxxxxxx> writes:
> On Tue, Mar 04, 2025 at 05:25:35PM +0530, Ritesh Harjani (IBM) wrote:
>> preadv2() was introduced in Linux 4.6. This patch adds support for
>> preadv2() to xfs_io.
>>
>> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
>> ---
>> configure.ac | 1 +
>> include/builddefs.in | 1 +
>> io/Makefile | 4 ++++
>> io/pread.c | 45 ++++++++++++++++++++++++++++---------------
>> m4/package_libcdev.m4 | 18 +++++++++++++++++
>> 5 files changed, 54 insertions(+), 15 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 8c76f398..658117ad 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -153,6 +153,7 @@ AC_PACKAGE_NEED_URCU_H
>> AC_PACKAGE_NEED_RCU_INIT
>>
>> AC_HAVE_PWRITEV2
>> +AC_HAVE_PREADV2
>
> I wonder, will we ever encounter a C library that has pwritev2 and /not/
> preadv2?
>
Sure make sense. I will use Makefile to detect if we have support of
HAVE_PWRITEV2 to also define HAVE_PREADV2 (instead of using autoconf).
>> AC_HAVE_COPY_FILE_RANGE
>> AC_NEED_INTERNAL_FSXATTR
>> AC_NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG
>> diff --git a/include/builddefs.in b/include/builddefs.in
>> index 82840ec7..a11d201c 100644
>> --- a/include/builddefs.in
>> +++ b/include/builddefs.in
>> @@ -94,6 +94,7 @@ ENABLE_SCRUB = @enable_scrub@
>> HAVE_ZIPPED_MANPAGES = @have_zipped_manpages@
>>
>> HAVE_PWRITEV2 = @have_pwritev2@
>> +HAVE_PREADV2 = @have_preadv2@
>> HAVE_COPY_FILE_RANGE = @have_copy_file_range@
>> NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
>> NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG = @need_internal_fscrypt_add_key_arg@
>> diff --git a/io/Makefile b/io/Makefile
>> index 8f835ec7..f8b19ac5 100644
>> --- a/io/Makefile
>> +++ b/io/Makefile
>> @@ -69,6 +69,10 @@ ifeq ($(HAVE_PWRITEV2),yes)
>> LCFLAGS += -DHAVE_PWRITEV2
>> endif
>>
>> +ifeq ($(HAVE_PREADV2),yes)
>> +LCFLAGS += -DHAVE_PREADV2
>> +endif
>> +
>> ifeq ($(HAVE_MAP_SYNC),yes)
>> LCFLAGS += -DHAVE_MAP_SYNC
>> endif
>> diff --git a/io/pread.c b/io/pread.c
>> index 62c771fb..782f2a36 100644
>> --- a/io/pread.c
>> +++ b/io/pread.c
>> @@ -162,7 +162,8 @@ static ssize_t
>> do_preadv(
>> int fd,
>> off_t offset,
>> - long long count)
>> + long long count,
>> + int preadv2_flags)
>
> Nit: ^ space before tab. There's a bunch more of thense, every
> time a "preadv2_flags" variable or parameter are declared.
>
Ohk, sorry about that. Let me fix these in v2.
>> {
>> int vecs = 0;
>> ssize_t oldlen = 0;
>> @@ -181,8 +182,14 @@ do_preadv(
>> } else {
>> vecs = vectors;
>> }
>> +#ifdef HAVE_PREADV2
>> + if (preadv2_flags)
>> + bytes = preadv2(fd, iov, vectors, offset, preadv2_flags);
>> + else
>> + bytes = preadv(fd, iov, vectors, offset);
>> +#else
>> bytes = preadv(fd, iov, vectors, offset);
>> -
>> +#endif
>
> Can we have the case that preadv2_flags!=0 and HAVE_PREADV2 isn't
> defined? If so, then there ought to be a warning about that.
>
That won't happen. Since case 'U' to take input flags is defined under
#ifdef HAVE_PREADV2. So if someone tries to set preadv2_flags using -U
when HAVE_PREADV2 is not true, it will go into default case where we
will use exitcode 1 and print command_usage()
> --D
>
Thanks for the quick review.
-ritesh
>> /* restore trimmed iov */
>> if (oldlen)
>> iov[vecs - 1].iov_len = oldlen;
>> @@ -195,12 +202,13 @@ do_pread(
>> int fd,
>> off_t offset,
>> long long count,
>> - size_t buffer_size)
>> + size_t buffer_size,
>> + int preadv2_flags)
>> {
>> if (!vectors)
>> return pread(fd, io_buffer, min(count, buffer_size), offset);
>>
>> - return do_preadv(fd, offset, count);
>> + return do_preadv(fd, offset, count, preadv2_flags);
>> }
>>
>> static int
>> @@ -210,7 +218,8 @@ read_random(
>> long long count,
>> long long *total,
>> unsigned int seed,
>> - int eof)
>> + int eof,
>> + int preadv2_flags)
>> {
>> off_t end, off, range;
>> ssize_t bytes;
>> @@ -234,7 +243,7 @@ read_random(
>> io_buffersize;
>> else
>> off = offset;
>> - bytes = do_pread(fd, off, io_buffersize, io_buffersize);
>> + bytes = do_pread(fd, off, io_buffersize, io_buffersize, preadv2_flags);
>> if (bytes == 0)
>> break;
>> if (bytes < 0) {
>> @@ -256,7 +265,8 @@ read_backward(
>> off_t *offset,
>> long long *count,
>> long long *total,
>> - int eof)
>> + int eof,
>> + int preadv2_flags)
>> {
>> off_t end, off = *offset;
>> ssize_t bytes = 0, bytes_requested;
>> @@ -276,7 +286,7 @@ read_backward(
>> /* Do initial unaligned read if needed */
>> if ((bytes_requested = (off % io_buffersize))) {
>> off -= bytes_requested;
>> - bytes = do_pread(fd, off, bytes_requested, io_buffersize);
>> + bytes = do_pread(fd, off, bytes_requested, io_buffersize, preadv2_flags);
>> if (bytes == 0)
>> return ops;
>> if (bytes < 0) {
>> @@ -294,7 +304,7 @@ read_backward(
>> while (cnt > end) {
>> bytes_requested = min(cnt, io_buffersize);
>> off -= bytes_requested;
>> - bytes = do_pread(fd, off, cnt, io_buffersize);
>> + bytes = do_pread(fd, off, cnt, io_buffersize, preadv2_flags);
>> if (bytes == 0)
>> break;
>> if (bytes < 0) {
>> @@ -318,14 +328,15 @@ read_forward(
>> long long *total,
>> int verbose,
>> int onlyone,
>> - int eof)
>> + int eof,
>> + int preadv2_flags)
>> {
>> ssize_t bytes;
>> int ops = 0;
>>
>> *total = 0;
>> while (count > 0 || eof) {
>> - bytes = do_pread(fd, offset, count, io_buffersize);
>> + bytes = do_pread(fd, offset, count, io_buffersize, preadv2_flags);
>> if (bytes == 0)
>> break;
>> if (bytes < 0) {
>> @@ -353,7 +364,7 @@ read_buffer(
>> int verbose,
>> int onlyone)
>> {
>> - return read_forward(fd, offset, count, total, verbose, onlyone, 0);
>> + return read_forward(fd, offset, count, total, verbose, onlyone, 0, 0);
>> }
>>
>> static int
>> @@ -371,6 +382,7 @@ pread_f(
>> int Cflag, qflag, uflag, vflag;
>> int eof = 0, direction = IO_FORWARD;
>> int c;
>> + int preadv2_flags = 0;
>>
>> Cflag = qflag = uflag = vflag = 0;
>> init_cvtnum(&fsblocksize, &fssectsize);
>> @@ -463,15 +475,18 @@ pread_f(
>> case IO_RANDOM:
>> if (!zeed) /* srandom seed */
>> zeed = time(NULL);
>> - c = read_random(file->fd, offset, count, &total, zeed, eof);
>> + c = read_random(file->fd, offset, count, &total, zeed, eof,
>> + preadv2_flags);
>> break;
>> case IO_FORWARD:
>> - c = read_forward(file->fd, offset, count, &total, vflag, 0, eof);
>> + c = read_forward(file->fd, offset, count, &total, vflag, 0, eof,
>> + preadv2_flags);
>> if (eof)
>> count = total;
>> break;
>> case IO_BACKWARD:
>> - c = read_backward(file->fd, &offset, &count, &total, eof);
>> + c = read_backward(file->fd, &offset, &count, &total, eof,
>> + preadv2_flags);
>> break;
>> default:
>> ASSERT(0);
>> diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
>> index 4ef7e8f6..5a1f748a 100644
>> --- a/m4/package_libcdev.m4
>> +++ b/m4/package_libcdev.m4
>> @@ -16,6 +16,24 @@ pwritev2(0, 0, 0, 0, 0);
>> AC_SUBST(have_pwritev2)
>> ])
>>
>> +#
>> +# Check if we have a preadv2 libc call (Linux)
>> +#
>> +AC_DEFUN([AC_HAVE_PREADV2],
>> + [ AC_MSG_CHECKING([for preadv2])
>> + AC_LINK_IFELSE(
>> + [ AC_LANG_PROGRAM([[
>> +#define _GNU_SOURCE
>> +#include <sys/uio.h>
>> + ]], [[
>> +preadv2(0, 0, 0, 0, 0);
>> + ]])
>> + ], have_preadv2=yes
>> + AC_MSG_RESULT(yes),
>> + AC_MSG_RESULT(no))
>> + AC_SUBST(have_preadv2)
>> + ])
>> +
>> #
>> # Check if we have a copy_file_range system call (Linux)
>> #
>> --
>> 2.48.1
>>
>>
