diff mbox series

[V2] LoongArch: Undef __NR_fstat and __NR_newfstatat.

Message ID 20240924032152.1346704-2-caiyinyu@loongson.cn
State New
Headers show
Series [V2] LoongArch: Undef __NR_fstat and __NR_newfstatat. | expand

Commit Message

caiyinyu Sept. 24, 2024, 3:21 a.m. UTC
In Linux 6.11, fstat and newfstatat are added back. To avoid the messy
usage of the fstat, newfstatat, and statx system calls, we will continue
using statx only in glibc, maintaining consistency with previous versions of
the LoongArch-specific glibc implementation.

Signed-off-by: caiyinyu <caiyinyu@loongson.cn>
Reviewed-by: Xi Ruoyao <xry111@xry111.site>
Reviewed-by: Miao Wang <shankerwangmiao@gmail.com>
Reviewed-by: Florian Weimer <fweimer@redhat.com>

V2:
According to Florian Weimer's suggest, undef __NR_fstat and
__NR_newfstatat in sysdeps/unix/sysv/linux/loongarch/fixup-asm-unistd.h.

V1: https://sourceware.org/pipermail/libc-alpha/2024-September/159932.html

---
 .../sysv/linux/loongarch/fixup-asm-unistd.h   | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/loongarch/fixup-asm-unistd.h

Comments

Florian Weimer Sept. 24, 2024, 8:04 a.m. UTC | #1
> In Linux 6.11, fstat and newfstatat are added back. To avoid the messy
> usage of the fstat, newfstatat, and statx system calls, we will continue
> using statx only in glibc, maintaining consistency with previous versions of
> the LoongArch-specific glibc implementation.

> +/* To avoid the messy usage of the fstat, newfstatat, and statx system calls, we
> +only use statx.  */

Comment and commit message mention statx twice, which find a bit
confusing.  I think the real issue is that generic code assumes that
fstat, newfstatat come first and statx comes later, and it's the other
way round for LoongArch.

> Reviewed-by: Florian Weimer <fweimer@redhat.com>

Sorry, I have not reviewed this.  Please do not add Reviewed-by:
proactively.  Perhaps you meant Suggested-by:?

Thanks,
Florian
Xi Ruoyao Sept. 24, 2024, 8:13 a.m. UTC | #2
On Tue, 2024-09-24 at 10:04 +0200, Florian Weimer wrote:
> > In Linux 6.11, fstat and newfstatat are added back. To avoid the messy
> > usage of the fstat, newfstatat, and statx system calls, we will continue
> > using statx only in glibc, maintaining consistency with previous versions of
> > the LoongArch-specific glibc implementation.
> 
> > +/* To avoid the messy usage of the fstat, newfstatat, and statx system calls, we
> > +only use statx.  */
> 
> Comment and commit message mention statx twice, which find a bit
> confusing.  I think the real issue is that generic code assumes that
> fstat, newfstatat come first and statx comes later, and it's the other
> way round for LoongArch.
> 
> > Reviewed-by: Florian Weimer <fweimer@redhat.com>
> 
> Sorry, I have not reviewed this.  Please do not add Reviewed-by:
> proactively.  Perhaps you meant Suggested-by:?

Generally R-b should be only used if the reviewer gives an explicit one.
But you can keep mine here.

Reviewed-by: Xi Ruoyao <xry111@xry111.site>
Miao Wang Sept. 24, 2024, 8:44 a.m. UTC | #3
> 2024年9月24日 16:13,Xi Ruoyao <xry111@xry111.site> 写道:
> 
> On Tue, 2024-09-24 at 10:04 +0200, Florian Weimer wrote:
>>> In Linux 6.11, fstat and newfstatat are added back. To avoid the messy
>>> usage of the fstat, newfstatat, and statx system calls, we will continue
>>> using statx only in glibc, maintaining consistency with previous versions of
>>> the LoongArch-specific glibc implementation.
>> 
>>> +/* To avoid the messy usage of the fstat, newfstatat, and statx system calls, we
>>> +only use statx.  */
>> 
>> Comment and commit message mention statx twice, which find a bit
>> confusing.  I think the real issue is that generic code assumes that
>> fstat, newfstatat come first and statx comes later, and it's the other
>> way round for LoongArch.
>> 
>>> Reviewed-by: Florian Weimer <fweimer@redhat.com>
>> 
>> Sorry, I have not reviewed this.  Please do not add Reviewed-by:
>> proactively.  Perhaps you meant Suggested-by:?
> 
> Generally R-b should be only used if the reviewer gives an explicit one.
> But you can keep mine here.
> 
> Reviewed-by: Xi Ruoyao <xry111@xry111.site>

Grateful if you can also remove my R-b since I'm not confident enough to
review this.

Cheers,

Miao Wang
Florian Weimer Sept. 24, 2024, 10:48 a.m. UTC | #4
* Miao Wang:

>> 2024年9月24日 16:13,Xi Ruoyao <xry111@xry111.site> 写道:
>> 
>> On Tue, 2024-09-24 at 10:04 +0200, Florian Weimer wrote:
>>>> In Linux 6.11, fstat and newfstatat are added back. To avoid the messy
>>>> usage of the fstat, newfstatat, and statx system calls, we will continue
>>>> using statx only in glibc, maintaining consistency with previous versions of
>>>> the LoongArch-specific glibc implementation.
>>> 
>>>> +/* To avoid the messy usage of the fstat, newfstatat, and statx system calls, we
>>>> +only use statx.  */
>>> 
>>> Comment and commit message mention statx twice, which find a bit
>>> confusing.  I think the real issue is that generic code assumes that
>>> fstat, newfstatat come first and statx comes later, and it's the other
>>> way round for LoongArch.
>>> 
>>>> Reviewed-by: Florian Weimer <fweimer@redhat.com>
>>> 
>>> Sorry, I have not reviewed this.  Please do not add Reviewed-by:
>>> proactively.  Perhaps you meant Suggested-by:?
>> 
>> Generally R-b should be only used if the reviewer gives an explicit one.
>> But you can keep mine here.
>> 
>> Reviewed-by: Xi Ruoyao <xry111@xry111.site>
>
> Grateful if you can also remove my R-b since I'm not confident enough to
> review this.

Adhemerval, I assume you don't have any objections to this approach,
given that we do the same thing on other architectures?

Thanks,
Florian
Miao Wang Sept. 24, 2024, 11:33 a.m. UTC | #5
> 2024年9月24日 18:48,Florian Weimer <fweimer@redhat.com> 写道:
> 
> * Miao Wang:
> 
>>> 2024年9月24日 16:13,Xi Ruoyao <xry111@xry111.site> 写道:
>>> 
>>> On Tue, 2024-09-24 at 10:04 +0200, Florian Weimer wrote:
>>>>> In Linux 6.11, fstat and newfstatat are added back. To avoid the messy
>>>>> usage of the fstat, newfstatat, and statx system calls, we will continue
>>>>> using statx only in glibc, maintaining consistency with previous versions of
>>>>> the LoongArch-specific glibc implementation.
>>>> 
>>>>> +/* To avoid the messy usage of the fstat, newfstatat, and statx system calls, we
>>>>> +only use statx.  */
>>>> 
>>>> Comment and commit message mention statx twice, which find a bit
>>>> confusing.  I think the real issue is that generic code assumes that
>>>> fstat, newfstatat come first and statx comes later, and it's the other
>>>> way round for LoongArch.
>>>> 
>>>>> Reviewed-by: Florian Weimer <fweimer@redhat.com>
>>>> 
>>>> Sorry, I have not reviewed this.  Please do not add Reviewed-by:
>>>> proactively.  Perhaps you meant Suggested-by:?
>>> 
>>> Generally R-b should be only used if the reviewer gives an explicit one.
>>> But you can keep mine here.
>>> 
>>> Reviewed-by: Xi Ruoyao <xry111@xry111.site>
>> 
>> Grateful if you can also remove my R-b since I'm not confident enough to
>> review this.
> 
> Adhemerval, I assume you don't have any objections to this approach,
> given that we do the same thing on other architectures?

No, I do not object to this approach. However, additionally, I also suggested
that we can also directly use fstat() like other architectures when the user
chooses to compile glibc targeted to kernel versions not below 6.10.6, and thus
no need to completely remove the definition of fstat() from loongarch. Since
in most cases, people will leave the targeted kernel version to its default
setting, I'm also not object to completely remove the definition of fstat()
and newfstatat()

Cheers,

Miao Wang
diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/loongarch/fixup-asm-unistd.h b/sysdeps/unix/sysv/linux/loongarch/fixup-asm-unistd.h
new file mode 100644
index 0000000000..0062756b5c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/loongarch/fixup-asm-unistd.h
@@ -0,0 +1,21 @@ 
+/* Regularize <asm/unistd.h> definitions.  LoongArch version.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* To avoid the messy usage of the fstat, newfstatat, and statx system calls, we
+only use statx.  */
+#undef __NR_fstat
+#undef __NR_newfstatat