* [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros @ 2025-09-01 23:14 Randy Dunlap 2025-09-02 6:58 ` Amir Goldstein 0 siblings, 1 reply; 6+ messages in thread From: Randy Dunlap @ 2025-09-01 23:14 UTC (permalink / raw) To: linux-fsdevel Cc: patches, Randy Dunlap, Amir Goldstein, Jeff Layton, Chuck Lever, Alexander Aring, Josef Bacik, Aleksa Sarai, Jan Kara, Christian Brauner, Matthew Wilcox, David Howells, linux-api 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 <rdunlap@infradead.org> --- Cc: Amir Goldstein <amir73il@gmail.com> Cc: Jeff Layton <jlayton@kernel.org> Cc: Chuck Lever <chuck.lever@oracle.com> Cc: Alexander Aring <alex.aring@gmail.com> Cc: Josef Bacik <josef@toxicpanda.com> Cc: Aleksa Sarai <cyphar@cyphar.com> 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 /* Flag for faccessat(2). */ #define AT_EACCESS 0x200 /* Test access permitted for ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros 2025-09-01 23:14 [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros Randy Dunlap @ 2025-09-02 6:58 ` Amir Goldstein 2025-09-02 21:31 ` Randy Dunlap 0 siblings, 1 reply; 6+ messages in thread From: Amir Goldstein @ 2025-09-02 6:58 UTC (permalink / raw) To: Randy Dunlap Cc: linux-fsdevel, patches, Jeff Layton, Chuck Lever, Alexander Aring, Josef Bacik, Aleksa Sarai, Jan Kara, Christian Brauner, Matthew Wilcox, David Howells, linux-api On Tue, Sep 2, 2025 at 1:14 AM Randy Dunlap <rdunlap@infradead.org> wrote: > > 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 <rdunlap@infradead.org> > --- > Cc: Amir Goldstein <amir73il@gmail.com> > Cc: Jeff Layton <jlayton@kernel.org> > Cc: Chuck Lever <chuck.lever@oracle.com> > Cc: Alexander Aring <alex.aring@gmail.com> > Cc: Josef Bacik <josef@toxicpanda.com> > Cc: Aleksa Sarai <cyphar@cyphar.com> > 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]: $ 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/ I don't know how to resolve the mess that glibc has created. 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 +*/ /* Flag for faccessat(2). */ #define AT_EACCESS 0x200 /* Test access permitted for ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros 2025-09-02 6:58 ` Amir Goldstein @ 2025-09-02 21:31 ` Randy Dunlap 2025-09-03 0:46 ` Randy Dunlap 0 siblings, 1 reply; 6+ messages in thread From: Randy Dunlap @ 2025-09-02 21:31 UTC (permalink / raw) To: Amir Goldstein Cc: linux-fsdevel, patches, Jeff Layton, Chuck Lever, Alexander Aring, Josef Bacik, Aleksa Sarai, Jan Kara, Christian Brauner, Matthew Wilcox, David Howells, linux-api Hi, On 9/1/25 11:58 PM, Amir Goldstein wrote: > On Tue, Sep 2, 2025 at 1:14 AM Randy Dunlap <rdunlap@infradead.org> wrote: >> >> 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 <rdunlap@infradead.org> >> --- >> Cc: Amir Goldstein <amir73il@gmail.com> >> Cc: Jeff Layton <jlayton@kernel.org> >> Cc: Chuck Lever <chuck.lever@oracle.com> >> Cc: Alexander Aring <alex.aring@gmail.com> >> Cc: Josef Bacik <josef@toxicpanda.com> >> Cc: Aleksa Sarai <cyphar@cyphar.com> >> 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. > $ 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/ > > I don't know how to resolve the mess that glibc has created. Yeah, I guess I don't either. > 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. > /* 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 <cyphar@cyphar.com> 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. 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. -- ~Randy ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros 2025-09-02 21:31 ` Randy Dunlap @ 2025-09-03 0:46 ` Randy Dunlap 2025-09-03 14:14 ` Amir Goldstein 0 siblings, 1 reply; 6+ messages in thread From: Randy Dunlap @ 2025-09-03 0:46 UTC (permalink / raw) To: Amir Goldstein Cc: linux-fsdevel, patches, Jeff Layton, Chuck Lever, Alexander Aring, Josef Bacik, Aleksa Sarai, Jan Kara, Christian Brauner, Matthew Wilcox, David Howells, linux-api On 9/2/25 2:31 PM, Randy Dunlap wrote: > Hi, > > On 9/1/25 11:58 PM, Amir Goldstein wrote: >> On Tue, Sep 2, 2025 at 1:14 AM Randy Dunlap <rdunlap@infradead.org> wrote: >>> >>> 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 <rdunlap@infradead.org> >>> --- >>> Cc: Amir Goldstein <amir73il@gmail.com> >>> Cc: Jeff Layton <jlayton@kernel.org> >>> Cc: Chuck Lever <chuck.lever@oracle.com> >>> Cc: Alexander Aring <alex.aring@gmail.com> >>> Cc: Josef Bacik <josef@toxicpanda.com> >>> Cc: Aleksa Sarai <cyphar@cyphar.com> >>> 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. > >> $ 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/ >> >> I don't know how to resolve the mess that glibc has created. > > Yeah, I guess I don't either. > >> 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. > > >> /* 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 <cyphar@cyphar.com> > 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. > > 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) 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. -- ~Randy ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros 2025-09-03 0:46 ` Randy Dunlap @ 2025-09-03 14:14 ` Amir Goldstein 2025-09-03 18:10 ` Randy Dunlap 0 siblings, 1 reply; 6+ messages in thread From: Amir Goldstein @ 2025-09-03 14:14 UTC (permalink / raw) To: Randy Dunlap Cc: linux-fsdevel, patches, Jeff Layton, Chuck Lever, Alexander Aring, Josef Bacik, Aleksa Sarai, Jan Kara, Christian Brauner, Matthew Wilcox, David Howells, linux-api On Wed, Sep 3, 2025 at 2:46 AM Randy Dunlap <rdunlap@infradead.org> wrote: > > > > On 9/2/25 2:31 PM, Randy Dunlap wrote: > > Hi, > > > > On 9/1/25 11:58 PM, Amir Goldstein wrote: > >> On Tue, Sep 2, 2025 at 1:14 AM Randy Dunlap <rdunlap@infradead.org> wrote: > >>> > >>> 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 <rdunlap@infradead.org> > >>> --- > >>> Cc: Amir Goldstein <amir73il@gmail.com> > >>> Cc: Jeff Layton <jlayton@kernel.org> > >>> Cc: Chuck Lever <chuck.lever@oracle.com> > >>> Cc: Alexander Aring <alex.aring@gmail.com> > >>> Cc: Josef Bacik <josef@toxicpanda.com> > >>> Cc: Aleksa Sarai <cyphar@cyphar.com> > >>> 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. > > > >> $ 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/ > >> > >> I don't know how to resolve the mess that glibc has created. > > > > Yeah, I guess I don't either. > > > >> 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. > > > > > >> /* 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 <cyphar@cyphar.com> > > 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. > > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros 2025-09-03 14:14 ` Amir Goldstein @ 2025-09-03 18:10 ` Randy Dunlap 0 siblings, 0 replies; 6+ messages in thread From: Randy Dunlap @ 2025-09-03 18:10 UTC (permalink / raw) To: Amir Goldstein Cc: linux-fsdevel, patches, Jeff Layton, Chuck Lever, Alexander Aring, Josef Bacik, Aleksa Sarai, Jan Kara, Christian Brauner, Matthew Wilcox, David Howells, linux-api On 9/3/25 7:14 AM, Amir Goldstein wrote: > On Wed, Sep 3, 2025 at 2:46 AM Randy Dunlap <rdunlap@infradead.org> wrote: >> >> >> >> On 9/2/25 2:31 PM, Randy Dunlap wrote: >>> Hi, >>> >>> On 9/1/25 11:58 PM, Amir Goldstein wrote: >>>> On Tue, Sep 2, 2025 at 1:14 AM Randy Dunlap <rdunlap@infradead.org> wrote: >>>>> >>>>> 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 <rdunlap@infradead.org> >>>>> --- >>>>> Cc: Amir Goldstein <amir73il@gmail.com> >>>>> Cc: Jeff Layton <jlayton@kernel.org> >>>>> Cc: Chuck Lever <chuck.lever@oracle.com> >>>>> Cc: Alexander Aring <alex.aring@gmail.com> >>>>> Cc: Josef Bacik <josef@toxicpanda.com> >>>>> Cc: Aleksa Sarai <cyphar@cyphar.com> >>>>> 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. >>> >>>> $ 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/ >>>> >>>> I don't know how to resolve the mess that glibc has created. >>> >>> Yeah, I guess I don't either. >>> >>>> 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. >>> >>> >>>> /* 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 <cyphar@cyphar.com> >>> 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. > >>> 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? OK, that works. I'll make a v3 like that. Thanks. -- ~Randy ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-03 18:10 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-01 23:14 [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros Randy Dunlap 2025-09-02 6:58 ` Amir Goldstein 2025-09-02 21:31 ` Randy Dunlap 2025-09-03 0:46 ` Randy Dunlap 2025-09-03 14:14 ` Amir Goldstein 2025-09-03 18:10 ` Randy Dunlap
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).