Re: [PATCH v3 1/2] tools/nolibc: fcntl: Add fallocate()

[David Laight <david.laight.linux@gmail.com>]

On Sat, 2 May 2026 05:00:06 +0200
Willy Tarreau <w@1wt.eu> wrote:

> On Fri, May 01, 2026 at 09:18:31AM +0100, David Laight wrote:
> > On Fri,  1 May 2026 01:41:24 +0900
> > Daniel Palmer <daniel@thingy.jp> wrote:
> >   
> > > Add fallocate().
> > > 
> > > Some special care is needed to put the offset and size
> > > into the syscall parameters for 32bit machines, x32,
> > > and mipsn32.
> > > 
> > > For x32 we can just check if the kernel long size is the
> > > same as off_t and use the same path as x86_64.
> > > 
> > > For mipsn32 we override the generic version and provide
> > > one that does the right thing.
> > > 
> > > Signed-off-by: Daniel Palmer <daniel@thingy.jp>
> > > ---
> > >  tools/include/nolibc/arch-mips.h | 11 +++++++++++
> > >  tools/include/nolibc/fcntl.h     | 33 ++++++++++++++++++++++++++++++++
> > >  tools/include/nolibc/sys.h       |  8 ++++++++
> > >  3 files changed, 52 insertions(+)
> > > 
> > > diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
> > > index 1400653c76c1..e4e42f2bcaf4 100644
> > > --- a/tools/include/nolibc/arch-mips.h
> > > +++ b/tools/include/nolibc/arch-mips.h
> > > @@ -6,6 +6,7 @@
> > >  
> > >  #ifndef _NOLIBC_ARCH_MIPS_H
> > >  #define _NOLIBC_ARCH_MIPS_H
> > > +#include <linux/unistd.h>
> > >  
> > >  #include "compiler.h"
> > >  #include "crt.h"
> > > @@ -256,6 +257,16 @@
> > >  	_arg4 ? -_num : _num;                                                 \
> > >  })
> > >  
> > > +/* The generic version of this will split offset and size for _ABIN32,
> > > + * override it and do the right thing here.
> > > + */
> > > +static __attribute__((unused))
> > > +int _sys_fallocate(int fd, int mode, off_t offset, off_t size)
> > > +{
> > > +	return __nolibc_syscall4(__NR_fallocate, fd, mode, offset, size);
> > > +}
> > > +#define _sys_fallocate _sys_fallocate
> > > +
> > >  #endif /* _ABIO32 */
> > >  
> > >  #ifndef NOLIBC_NO_RUNTIME
> > > diff --git a/tools/include/nolibc/fcntl.h b/tools/include/nolibc/fcntl.h
> > > index 014910a8e928..dbc99188a49e 100644
> > > --- a/tools/include/nolibc/fcntl.h
> > > +++ b/tools/include/nolibc/fcntl.h
> > > @@ -14,6 +14,9 @@
> > >  #include "types.h"
> > >  #include "sys.h"
> > >  
> > > +/* For fallocate() modes */
> > > +#include <linux/falloc.h>
> > > +
> > >  /*
> > >   * int openat(int dirfd, const char *path, int flags[, mode_t mode]);
> > >   */
> > > @@ -80,4 +83,34 @@ int creat(const char *path, mode_t mode)
> > >  	return open(path, O_CREAT | O_WRONLY | O_TRUNC, mode);
> > >  }
> > >  
> > > +/*
> > > + * int fallocate(int fd, int mode, off_t offset, off_t size);
> > > + */
> > > +
> > > +#if !defined(_sys_fallocate)
> > > +static __attribute__((unused))
> > > +int _sys_fallocate(int fd, int mode, off_t offset, off_t size)
> > > +{
> > > +	/*
> > > +	 * For 32 bit machines __kernel_long_t will be 4, off_t will be 8
> > > +	 * and we need to split offset and size, for 64 machines we can use
> > > +	 * the values as-is.
> > > +	 */
> > > +	const bool offsetsz_two_args = sizeof(__kernel_long_t) != sizeof(off_t);  
> > 
> > I don't think you care about the size of off_t.
> > Were it to be 4 the code would be badly wrong.
> >   
> > > +
> > > +	if (offsetsz_two_args)
> > > +		return __nolibc_syscall6(__NR_fallocate, fd, mode,
> > > +			__NOLIBC_LLARGPART(offset, 0), __NOLIBC_LLARGPART(offset, 1),
> > > +			__NOLIBC_LLARGPART(size, 0), __NOLIBC_LLARGPART(size, 1));
> > > +	else
> > > +		return __nolibc_syscall4(__NR_fallocate, fd, mode, offset, size);
> > > +}  
> > 
> > The above might be more readable as:
> > 	if (sizeof(__kernel_long_t) == 8)
> > 		/* 64 bit, values fit in single arguments */
> > 		return __nolibc_syscall4(__NR_fallocate, fd, mode, offset, size);
> > 
> > 	/* 32 bit, values need splitting, order depends on endianness */
> > 	/* This test for endianness doesn't rely on any pre-processor defines */
> > 	if (({union {int x; char c;} u; u.x = 1; u.c;}))
> > 		/* Little endian */
> > 		return __nolibc_syscall6(__NR_fallocate, fd, mode,
> > 			offset, offset >> 32, size, size >> 32);
> > 	/* Big endian */
> > 	return __nolibc_syscall6(__NR_fallocate, fd, mode,
> > 		offset >> 32, offset, size >> 32, size);  
> 
> Honestly David, I find Daniel's version way more readable :-) Precisely
> because the repeated variations are abstracted with this more readable
> macro. If it was used only once I could possibly agree. Even the
> endianness test is hard to read, better rely on __BYTE_ORDER__ for
> this.

I did say 'might' :-) and I should probably have used __BYTE_ORDER__.

Looking again, the code is trying to copy what the compiler generates
when it passes a 64bit quantity on stack or in 2 registers.
So f(a, b, c, d) is traditionally 'push d; push c; push b, push a; call f'.
With the normal 'stack grows down' this gives (in increasing addresses):
	return address
	a
	b
	c
	d
If 'b,c' is replaced by a 64bit value then you want the stack memory
to contain the correct representation of a 64bit value.
So for LE you need to pass the low part before the high part.

But there is one architecture (parisc) where the stack goes the other way.
It is BE (I believe), but would need the LE argument order.

I think all the conditionals could be moved out of the fallocate code.
I'm not sure of the exact pre-processor conditionals but something like:

#if (__BITS_PER_LONG == 64) || defined(x86_x32) || defined(mips_n32)
#define __NOLIBC_ARG64(x) (x)
#else
/* The on-stack data has to be in the natural order for a 64bit value. */
#if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN_) ^ defined(STACK_GROWS_UP)
#define __NOLIBC_ARG64(x) ((int)(x)), ((int)((long long)(x) >> 32))
#else
#define __NOLIBC_ARG64(x) ((int)((long long)(x) >> 32)), ((int)(x))
#endif
#endif

Then (if I've got it right):
static __attribute__((unused))
int _sys_fallocate(int fd, int mode, off_t offset, off_t size)
{
	return _syscall(__NR_fallocate, fd, mode, __NOLIBC_ARG64(offset),
			__NOLIBC_ARG64(size));
}

There is the other problem that arm32 (and maybe others) requires the 64bit
variable be aligned in the stack frame.
So f(int a, long long b) is effectively f(int a, int pad, long long b).
(This usually causes grief with lseek().)
So a pad might be needed for some system calls on some 32bit architectures.
This could be done with something that expands to '' or '0,'.

	David
   

> 
> Cheers,
> Willy