On Wed, Sep 3, 2025 at 2:46 AM Randy Dunlap [off-list ref] wrote:

On 9/2/25 2:31 PM, Randy Dunlap wrote:

quoted

Hi,

On 9/1/25 11:58 PM, Amir Goldstein wrote:

quoted

On Tue, Sep 2, 2025 at 1:14 AM Randy Dunlap [off-list ref] wrote:

quoted

Define the RENAME_* and AT_RENAME_* macros exactly the same as in
recent glibc <stdio.h> so that duplicate definition build errors in
both samples/watch_queue/watch_test.c and samples/vfs/test-statx.c
no longer happen. When they defined in exactly the same way in
multiple places, the build errors are prevented.

Defining only the AT_RENAME_* macros is not sufficient since they
depend on the RENAME_* macros, which may not be defined when the
AT_RENAME_* macros are used.

Build errors being fixed:

for samples/vfs/test-statx.c:

In file included from ../samples/vfs/test-statx.c:23:
usr/include/linux/fcntl.h:159:9: warning: ‘AT_RENAME_NOREPLACE’ redefined
  159 | #define AT_RENAME_NOREPLACE     0x0001
In file included from ../samples/vfs/test-statx.c:13:
/usr/include/stdio.h:171:10: note: this is the location of the previous definition
  171 | # define AT_RENAME_NOREPLACE RENAME_NOREPLACE
usr/include/linux/fcntl.h:160:9: warning: ‘AT_RENAME_EXCHANGE’ redefined
  160 | #define AT_RENAME_EXCHANGE      0x0002
/usr/include/stdio.h:173:10: note: this is the location of the previous definition
  173 | # define AT_RENAME_EXCHANGE RENAME_EXCHANGE
usr/include/linux/fcntl.h:161:9: warning: ‘AT_RENAME_WHITEOUT’ redefined
  161 | #define AT_RENAME_WHITEOUT      0x0004
/usr/include/stdio.h:175:10: note: this is the location of the previous definition
  175 | # define AT_RENAME_WHITEOUT RENAME_WHITEOUT

for samples/watch_queue/watch_test.c:

In file included from usr/include/linux/watch_queue.h:6,
                 from ../samples/watch_queue/watch_test.c:19:
usr/include/linux/fcntl.h:159:9: warning: ‘AT_RENAME_NOREPLACE’ redefined
  159 | #define AT_RENAME_NOREPLACE     0x0001
In file included from ../samples/watch_queue/watch_test.c:11:
/usr/include/stdio.h:171:10: note: this is the location of the previous definition
  171 | # define AT_RENAME_NOREPLACE RENAME_NOREPLACE
usr/include/linux/fcntl.h:160:9: warning: ‘AT_RENAME_EXCHANGE’ redefined
  160 | #define AT_RENAME_EXCHANGE      0x0002
/usr/include/stdio.h:173:10: note: this is the location of the previous definition
  173 | # define AT_RENAME_EXCHANGE RENAME_EXCHANGE
usr/include/linux/fcntl.h:161:9: warning: ‘AT_RENAME_WHITEOUT’ redefined
  161 | #define AT_RENAME_WHITEOUT      0x0004
/usr/include/stdio.h:175:10: note: this is the location of the previous definition
  175 | # define AT_RENAME_WHITEOUT RENAME_WHITEOUT

Fixes: b4fef22c2fb9 ("uapi: explain how per-syscall AT_* flags should be allocated")
Signed-off-by: Randy Dunlap <redacted>
---
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever <redacted>
Cc: Alexander Aring <alex.aring@gmail.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: Aleksa Sarai <redacted>
Cc: Jan Kara <jack@suse.cz>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: David Howells <dhowells@redhat.com>
CC: linux-api@vger.kernel.org
To: linux-fsdevel@vger.kernel.org

 include/uapi/linux/fcntl.h |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--- linux-next-20250819.orig/include/uapi/linux/fcntl.h
+++ linux-next-20250819/include/uapi/linux/fcntl.h

@@ -156,9 +156,12 @@
  */

 /* Flags for renameat2(2) (must match legacy RENAME_* flags). */
-#define AT_RENAME_NOREPLACE    0x0001
-#define AT_RENAME_EXCHANGE     0x0002
-#define AT_RENAME_WHITEOUT     0x0004
+# define RENAME_NOREPLACE (1 << 0)
+# define AT_RENAME_NOREPLACE RENAME_NOREPLACE
+# define RENAME_EXCHANGE (1 << 1)
+# define AT_RENAME_EXCHANGE RENAME_EXCHANGE
+# define RENAME_WHITEOUT (1 << 2)
+# define AT_RENAME_WHITEOUT RENAME_WHITEOUT

This solution, apart from being terribly wrong (adjust the source to match
to value of its downstream copy), does not address the issue that Mathew
pointed out on v1 discussion [1]:

I didn't forget or ignore this.
If the macros have the same values (well, not just values but also the
same text), then I don't see why it matters whether they are in some older
version of glibc.

quoted

$ grep -r AT_RENAME_NOREPLACE /usr/include
/usr/include/linux/fcntl.h:#define AT_RENAME_NOREPLACE  0x0001

It's not in stdio.h at all.  This is with libc6 2.41-10

[1] https://lore.kernel.org/linux-fsdevel/aKxfGix_o4glz8-Z@casper.infradead.org/ (local)

I don't know how to resolve the mess that glibc has created.

Yeah, I guess I don't either.

quoted

Perhaps like this:

diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index f291ab4f94ebc..dde14fa3c2007 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h

@@ -155,10 +155,16 @@
  * as possible, so we can use them for generic bits in the future if necessary.
  */

-/* Flags for renameat2(2) (must match legacy RENAME_* flags). */
-#define AT_RENAME_NOREPLACE    0x0001
-#define AT_RENAME_EXCHANGE     0x0002
-#define AT_RENAME_WHITEOUT     0x0004
+/*
+ * The legacy renameat2(2) RENAME_* flags are conceptually also

syscall-specific
+ * flags, so it could makes sense to create the AT_RENAME_* aliases
for them and
+ * maybe later add support for generic AT_* flags to this syscall.
+ * However, following a mismatch of definitions in glibc and since no
kernel code
+ * currently uses the AT_RENAME_* aliases, we leave them undefined here.
+#define AT_RENAME_NOREPLACE    RENAME_NOREPLACE
+#define AT_RENAME_EXCHANGE     RENAME_EXCHANGE
+#define AT_RENAME_WHITEOUT     RENAME_WHITEOUT
+*/

Well, we do have samples/ code that uses fcntl.h (indirectly; maybe
that can be fixed).
See the build errors in the patch description.

quoted

 /* Flag for faccessat(2). */
 #define AT_EACCESS             0x200   /* Test access permitted for

With this patch (your suggestion above):

IF a userspace program in samples/ uses <uapi/linux/fcntl.h> without
using <stdio.h>, [yes, I created one to test this] and without using
<uapi/linux/fs.h> then the build fails with similar build errors:

../samples/watch_queue/watch_nostdio.c: In function ‘consumer’:
../samples/watch_queue/watch_nostdio.c:33:32: error: ‘RENAME_NOREPLACE’ undeclared (first use in this function)
   33 |                         return RENAME_NOREPLACE;
../samples/watch_queue/watch_nostdio.c:33:32: note: each undeclared identifier is reported only once for each function it appears in
../samples/watch_queue/watch_nostdio.c:37:32: error: ‘RENAME_EXCHANGE’ undeclared (first use in this function)
   37 |                         return RENAME_EXCHANGE;
../samples/watch_queue/watch_nostdio.c:41:32: error: ‘RENAME_WHITEOUT’ undeclared (first use in this function)
   41 |                         return RENAME_WHITEOUT;

This build succeeds with my version 1 patch (full defining of both
RENAME_* and AT_RENAME_* macros). It fails with the patch that you suggested
above.

OK, here's what I propose.

a. remove the unused and (sort of) recently added AT_RENAME_* macros
in include/uapi/linux/fcntl.h. Nothing in the kernel tree uses them.
This is:

commit b4fef22c2fb9
Author: Aleksa Sarai [off-list ref]
Date:   Wed Aug 28 20:19:42 2024 +1000
    uapi: explain how per-syscall AT_* flags should be allocated

These macros should have never been added here IMO.
Just putting them somewhere as examples (in comments) would be OK.

I agree.
I did not get this patch from Aleksa,
but I proposed something similar above.

quoted

This alone fixes all of the build errors in samples/ that I originally
reported.

b. if a userspace program wants to use the RENAME_* macros, it should
#include <linux/fs.h> instead of <linux/fcntl.h>.

This fixes the "contrived" build error that I manufactured.

Note that some programs in tools/ do use AT_RENAME_* (all 3 macros)
but they define those macros locally.

And after more testing, this is what I think works:

a. remove all of the AT_RENAME-* macros from <uapi/linux/fcntl.h>
   (as above)

ok.

b. put the AT_RENAME_* macros into <uapi/linux/fs.h> like so:

+/* Flags for renameat2(2) (must match legacy RENAME_* flags). */
+# define AT_RENAME_NOREPLACE RENAME_NOREPLACE
+# define AT_RENAME_EXCHANGE RENAME_EXCHANGE
+# define AT_RENAME_WHITEOUT RENAME_WHITEOUT

so that they match what is in upstream glibc stdio.h, hence not
causing duplicate definition errors.

Disagree.
We do not need to define them at all.

The *only* reason we defined them in fcntl.h is so the
definition will be together with the rest of the AT_ flags.
Now we change that to a comment, but there is no reason to
define them at fs.h. Why would we need to do that?

Thanks,
Amir.