From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7F32B2E92C3 for ; Wed, 3 Sep 2025 14:14:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756908900; cv=none; b=PVlCLQiNfq0dzntlccAPHK+2z3s6zoWGkhoqtbJuejF6WZjxBvGXKnOrzuDIRDGFSymcmUNhrpJfzQ/L2UaXyrzwjUhPnaYyGUj97Giv07jqkO8F+pkmMnFayGTuafxsTcqoDoMFdoaxxdMIcIhTuyrozes8QFrDZ9KU+Q14eS8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756908900; c=relaxed/simple; bh=7z0O2nG5P8sF45YF4K38XBklUL2KhQ6VXBcBADLg07A=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=G4NKNiN0O3F5a4zHcFhYXdUcoA5WRDvw4awJp/pwYMMrbrEGW/jgTRxdg03LoNeJnwIw0ydIOhZ0RiFAPd7sy25+Wqgpud5f0QCD0PKkbQ5nGFkHJH9XeTtkABE197Jw/kNpe4hWm0M6MLl37K99/Xxa4DWt22sScVb0eGB6O/w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=TrZkpimH; arc=none smtp.client-ip=209.85.208.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="TrZkpimH" Received: by mail-ed1-f42.google.com with SMTP id 4fb4d7f45d1cf-61cc281171cso11385137a12.0 for ; Wed, 03 Sep 2025 07:14:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1756908897; x=1757513697; darn=lists.linux.dev; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=lHLCbxm2qevOsyTY7hfbOkOW4ktOmeHjBjDsV0iAsNU=; b=TrZkpimH/KZ5qFTZNIaC4q5iRPNKHwSA3Aiv/zMx/VTUAKcwXg7HBxsfmCCk2er27c OgxRO5gQOWzV8XlijfTSWZRBcDucJmfIZDz3UB9zlNgKEygjMrSCM3emGC0hEIrjL2Vm 0KqfLE++rdgLkLh330Scvk9ygfaCDsXJRsfBrdyu9taQlMbavNBTlB1SlAOr4fqwVDst 5oZhJaAjMVqjokaBwSiv+m+b65t/oARy7p05X1n8rf389MlOKQk/GwlUw7szvy9fDRTo hj1z2hU7Ywp1w60wifOZXpfff+sicOwlpiwQ8UxTuQqjw6MUh6+wnvPmEAx3FRHtvpsu kt5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1756908897; x=1757513697; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=lHLCbxm2qevOsyTY7hfbOkOW4ktOmeHjBjDsV0iAsNU=; b=Z+Mr88qNEDjXCfP+77VcmgPeSV5w2OzH8V40kKRTiQB9x6Tgb0myOvcu9O/UEj9I7Q 1RheWjszsUv1wFs0Sqa8hByPOwUIz8B68mQBwe/JlWLBaGUSXMJ3zauWK+72SEhvoKDq UfFeE1kpqanIsHsAlgXquY2bLbMkTtNscwxekBUIMkB+XhN+PD0MbntkO/p6sf+Ohdph F0IkKmX/aMJRSlhC2NskumZIsPgsPTylWPzSUzGBtTKGAd2DuwekMat318h89+uSCge5 PE+3mkqInnoZZFYuxN7S6nPp+uhHR2f1zwyS0y6OilvqiriR7U1iK/r1mv86VSQYupZv eJPQ== X-Forwarded-Encrypted: i=1; AJvYcCXUP5emDCwYCeQPh2eTBy2OpM0QZQqxFJVW9mRDSxHpoWoI0n8oVnic9LSLvDiwCgMDBMYcL5Ca@lists.linux.dev X-Gm-Message-State: AOJu0YzCT69X8t69KNRHuJ3CbvEfhMpyda6olB3i4+NRZ58/bq9poyJu iHtoPlKk2cx535wdhmsr16MGgHKEUISzc2exV3LTVOUrV+sTMc45F9xmqU5noZaYO11+T8rXn1x F3WeJ7BKLX2QWpcbZ8sLQNVio8fyolbs= X-Gm-Gg: ASbGncvQFfmfUOocu+un452qTL9GE7hXACaI4sTz+SjLeWeRmS3X68Ay0WrcI9rcNGF tskOd6S373oePO92RAVGVpRqa0mJ92ei/1MUwTGLWEcPH3n4m+dA5Em9Vdr6mriqaQAQQpv/8Ni /eY2atSCXHeDqh0DWT0nm9AIreqXEbAoSZlWTpBYcJpzgeLfzWHGS5jJk3bXnWjgNQVuB0E+peP H73Czt7quzPzw/uDg== X-Google-Smtp-Source: AGHT+IExdsquEhg4Y7gn/T31I32x6vp4PP00/sIHuQwzHhGFjnQjuw1ubn2EZCbVr7BAg8ZN3AP7lol3/uimD46c03I= X-Received: by 2002:a05:6402:51cb:b0:618:2733:1a52 with SMTP id 4fb4d7f45d1cf-61d26997500mr13189130a12.8.1756908896472; Wed, 03 Sep 2025 07:14:56 -0700 (PDT) Precedence: bulk X-Mailing-List: patches@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20250901231457.1179748-1-rdunlap@infradead.org> <5ff4dfe2-271f-4967-bb45-ad59614edc37@infradead.org> In-Reply-To: From: Amir Goldstein Date: Wed, 3 Sep 2025 16:14:45 +0200 X-Gm-Features: Ac12FXyjWPU17SHaCP67thoU9lOuY_REmGjEqZhmjb3HYVjgu2ZGEt5MECy2X6E Message-ID: Subject: Re: [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros To: Randy Dunlap Cc: linux-fsdevel@vger.kernel.org, patches@lists.linux.dev, Jeff Layton , Chuck Lever , Alexander Aring , Josef Bacik , Aleksa Sarai , Jan Kara , Christian Brauner , Matthew Wilcox , David Howells , linux-api@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Sep 3, 2025 at 2:46=E2=80=AFAM Randy Dunlap = 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=E2=80=AFAM Randy Dunlap wrote: > >>> > >>> Define the RENAME_* and AT_RENAME_* macros exactly the same as in > >>> recent glibc 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: =E2=80=98AT_RENAME_NOREPLAC= E=E2=80=99 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 previo= us definition > >>> 171 | # define AT_RENAME_NOREPLACE RENAME_NOREPLACE > >>> usr/include/linux/fcntl.h:160:9: warning: =E2=80=98AT_RENAME_EXCHANGE= =E2=80=99 redefined > >>> 160 | #define AT_RENAME_EXCHANGE 0x0002 > >>> /usr/include/stdio.h:173:10: note: this is the location of the previo= us definition > >>> 173 | # define AT_RENAME_EXCHANGE RENAME_EXCHANGE > >>> usr/include/linux/fcntl.h:161:9: warning: =E2=80=98AT_RENAME_WHITEOUT= =E2=80=99 redefined > >>> 161 | #define AT_RENAME_WHITEOUT 0x0004 > >>> /usr/include/stdio.h:175:10: note: this is the location of the previo= us 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: =E2=80=98AT_RENAME_NOREPLAC= E=E2=80=99 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 previo= us definition > >>> 171 | # define AT_RENAME_NOREPLACE RENAME_NOREPLACE > >>> usr/include/linux/fcntl.h:160:9: warning: =E2=80=98AT_RENAME_EXCHANGE= =E2=80=99 redefined > >>> 160 | #define AT_RENAME_EXCHANGE 0x0002 > >>> /usr/include/stdio.h:173:10: note: this is the location of the previo= us definition > >>> 173 | # define AT_RENAME_EXCHANGE RENAME_EXCHANGE > >>> usr/include/linux/fcntl.h:161:9: warning: =E2=80=98AT_RENAME_WHITEOUT= =E2=80=99 redefined > >>> 161 | #define AT_RENAME_WHITEOUT 0x0004 > >>> /usr/include/stdio.h:175:10: note: this is the location of the previo= us 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 > >>> --- > >>> Cc: Amir Goldstein > >>> Cc: Jeff Layton > >>> Cc: Chuck Lever > >>> Cc: Alexander Aring > >>> Cc: Josef Bacik > >>> Cc: Aleksa Sarai > >>> Cc: Jan Kara > >>> Cc: Christian Brauner > >>> Cc: Matthew Wilcox > >>> Cc: David Howells > >>> 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 m= atch > >> to value of its downstream copy), does not address the issue that Math= ew > >> 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 ol= der > > 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.infr= adead.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 he= re. > >> +#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 without > > using , [yes, I created one to test this] and without using > > then the build fails with similar build errors: > > > > ../samples/watch_queue/watch_nostdio.c: In function =E2=80=98consumer= =E2=80=99: > > ../samples/watch_queue/watch_nostdio.c:33:32: error: =E2=80=98RENAME_NO= REPLACE=E2=80=99 undeclared (first use in this function) > > 33 | return RENAME_NOREPLACE; > > ../samples/watch_queue/watch_nostdio.c:33:32: note: each undeclared ide= ntifier is reported only once for each function it appears in > > ../samples/watch_queue/watch_nostdio.c:37:32: error: =E2=80=98RENAME_EX= CHANGE=E2=80=99 undeclared (first use in this function) > > 37 | return RENAME_EXCHANGE; > > ../samples/watch_queue/watch_nostdio.c:41:32: error: =E2=80=98RENAME_WH= ITEOUT=E2=80=99 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 sugg= ested > > 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 > > 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 instead of . > > > > 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 > (as above) ok. > > b. put the AT_RENAME_* macros into 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.