[v3,02/29] Prepare for the addition of the <sys/pagesize.h> header
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
Commit Message
Add a stub version of <bits/pagesize.h>. This header is similar to
<bits/wordsize.h> in that every port must define its own version.
Add <sys/pagesize.h>, which will become the installed version
of this header.
---
bits/pagesize.h | 10 ++++++++++
include/sys/pagesize.h | 1 +
misc/sys/pagesize.h | 32 ++++++++++++++++++++++++++++++++
3 files changed, 43 insertions(+)
create mode 100644 bits/pagesize.h
create mode 100644 include/sys/pagesize.h
create mode 100644 misc/sys/pagesize.h
Comments
On 2024-09-29 08:49, Florian Weimer wrote:
> +#define PAGE_SIZE_MIN (1UL << __GLIBC_PAGE_SHIFT_MIN)
> +#define PAGE_SIZE_MAX (1UL << __GLIBC_PAGE_SHIFT_MAX)
Shouldn't these have the same type as sysconf (_SC_PAGESIZE)? It seems
odd to introduce yet another type for page sizes.
(I write this as a longtime fan of signed types due to the better
checking for them, so yeah, I'm biased....)
* Paul Eggert:
> On 2024-09-29 08:49, Florian Weimer wrote:
>> +#define PAGE_SIZE_MIN (1UL << __GLIBC_PAGE_SHIFT_MIN)
>> +#define PAGE_SIZE_MAX (1UL << __GLIBC_PAGE_SHIFT_MAX)
>
> Shouldn't these have the same type as sysconf (_SC_PAGESIZE)? It seems
> odd to introduce yet another type for page sizes.
Well, getpagesize returns an int, so there's not much consistency to be
had here.
I can turn it into 1L if you prefer that, but byte counts are generally
measured in size_t …
Thanks,
Florian
On 2024-09-29 12:47, Florian Weimer wrote:
> * Paul Eggert:
>
>> On 2024-09-29 08:49, Florian Weimer wrote:
>>> +#define PAGE_SIZE_MIN (1UL << __GLIBC_PAGE_SHIFT_MIN)
>>> +#define PAGE_SIZE_MAX (1UL << __GLIBC_PAGE_SHIFT_MAX)
>>
>> Shouldn't these have the same type as sysconf (_SC_PAGESIZE)? It seems
>> odd to introduce yet another type for page sizes.
>
> Well, getpagesize returns an int, so there's not much consistency to be
> had here.
That's why POSIX removed getpagesize way back in 2001. The function
shouldn't be used in portable code and is not a good model for page sizes.
> I can turn it into 1L if you prefer that, but byte counts are generally
> measured in size_t …
I do prefer it, partly because of sysconf (_SC_PAGE_SIZE), partly
because of the general trend in GNU utilities to move away from size_t
for sizes (admittedly this is a long road to travel).
* Paul Eggert:
> On 2024-09-29 12:47, Florian Weimer wrote:
>> * Paul Eggert:
>>
>>> On 2024-09-29 08:49, Florian Weimer wrote:
>>>> +#define PAGE_SIZE_MIN (1UL << __GLIBC_PAGE_SHIFT_MIN)
>>>> +#define PAGE_SIZE_MAX (1UL << __GLIBC_PAGE_SHIFT_MAX)
>>>
>>> Shouldn't these have the same type as sysconf (_SC_PAGESIZE)? It seems
>>> odd to introduce yet another type for page sizes.
>> Well, getpagesize returns an int, so there's not much consistency to
>> be
>> had here.
>
> That's why POSIX removed getpagesize way back in 2001. The function
> shouldn't be used in portable code and is not a good model for page
> sizes.
It's a pitty because getpagesize is easier to optimize than sysconf
(_SC_PAGESIZE). But we can make both work. (The series only has the
constant page size optimization, it does not turn sysconf (_SC_PAGESIZE)
into __getpagesize () or local variable access.)
>> I can turn it into 1L if you prefer that, but byte counts are generally
>> measured in size_t …
>
> I do prefer it, partly because of sysconf (_SC_PAGE_SIZE), partly
> because of the general trend in GNU utilities to move away from size_t
> for sizes (admittedly this is a long road to travel).
I don't agree with this rationale, but I really want
roundup (offset, PAGE_SIZE_MAX)
to do the right thing if offset is off64_t, and that requires using
unsigned long long int on 32-bit—or a signed type. So the latter it is.
I'll post a v2.
Thanks,
Florian
On Tue, 1 Oct 2024, Florian Weimer wrote:
> > That's why POSIX removed getpagesize way back in 2001. The function
> > shouldn't be used in portable code and is not a good model for page
> > sizes.
>
> It's a pitty because getpagesize is easier to optimize than sysconf
> (_SC_PAGESIZE). But we can make both work. (The series only has the
> constant page size optimization, it does not turn sysconf (_SC_PAGESIZE)
> into __getpagesize () or local variable access.)
Why do you think `getpagesize' is easier to optimise than `sysconf
(_SC_PAGESIZE)'?
I can certainly envisage `sysconf' being a static inline function in
<unistd.h> that handles all the known (or important enough) compile-time
constants itself where the identifier of the parameter requested is also a
constant, while deferring to a library call for everything else.
Maciej
On 2024-10-01 09:56, Maciej W. Rozycki wrote:
> I can certainly envisage `sysconf' being a static inline function in
> <unistd.h>
It couldn't be static, as then people couldn't call it from an extern
inline function (as per ISO C24 §6.7.5 ¶2).
It could be a macro.
On Tue, 1 Oct 2024, Paul Eggert wrote:
> > I can certainly envisage `sysconf' being a static inline function in
> > <unistd.h>
>
> It couldn't be static, as then people couldn't call it from an extern inline
> function (as per ISO C24 §6.7.5 ¶2).
And why is it going to be a problem? An extern inline function is either
inlined (in which case so are any calls to static inline functions within)
or the call site defers to an external definition (in which case the body
is discarded, so it doesn't matter what it calls). The ISO C24 clause you
refer says:
"Function specifiers shall be used only in the declaration of an
identifier for a function."
which doesn't seem relevant to me. Clearly I'm missing something, so
please elaborate.
> It could be a macro.
Ouch!
Maciej
On 2024-10-01 11:22, Maciej W. Rozycki wrote:
> And why is it going to be a problem?
It's in a standard as a constraint, which means standard compilers are
required to issue a diagnostic, unfortunately.
> The ISO C24 clause you
> refer says:
Oops, I meant to cite paragraph 3 not paragraph 2. Sorry about that.
Paragraph 3 says, "An inline definition of a function with external
linkage shall not ... contain, anywhere in the tokens making up the
function definition, a reference to an identifier with internal
linkage." This means a user-defined extern inline function cannot call a
static function.
I suppose we could do something like this:
#if __GNUC_PREREQ (3,2) || __glibc_has_attribute (__always_inline__)
static __always_inline long int
__glibc_sysconf (int __name)
{
... optimizations go here ...
}
# define sysconf(name) __glibc_sysconf (name)
#endif
This would support the C standard rules, so long as GCC is smart enough
to not issue a diagnostic about this sort of thing (is it?).
* Paul Eggert:
> On 2024-10-01 11:22, Maciej W. Rozycki wrote:
>> And why is it going to be a problem?
>
> It's in a standard as a constraint, which means standard compilers are
> required to issue a diagnostic, unfortunately.
We already have __extern_inline in <sys/cdefs.h> for that. It's used
for things like bsearch. This patch adds more inline functions like
that:
[PATCH v3 29/29] Optimize various ways to obtain the page size using
<bits/pagesize.h>
<https://inbox.sourceware.org/libc-alpha/52aa57f9ba1971262b8aaa5c705b2a260e6c09eb.1727624528.git.fweimer@redhat.com/>
The tricky part is that we want to tell GCC that sysconf (_SC_PAGE_SIZE)
is a constant, but sysconf in general is not __attribute__
((__const__)). The easiest way to implement that is to call
__getpagesize, which has the right attribute. The patch doesn't
implement that, though.
Thanks,
Florian
On 2024-10-01 15:54, Florian Weimer wrote:
> We already have __extern_inline in <sys/cdefs.h> for that.
Thanks, didn't know that.
> The tricky part is that we want to tell GCC that sysconf (_SC_PAGE_SIZE)
> is a constant, but sysconf in general is not __attribute__
> ((__const__)). The easiest way to implement that is to call
> __getpagesize, which has the right attribute. The patch doesn't
> implement that, though.
Can we do something like the following to make sysconf (_SC_PAGESIZE)
effectively const?
#ifdef __extern_always_inline
__extern_always_inline __attribute_const__ long int
__glibc_sysconf_const (int __name)
{
return sysconf (__name);
}
# define sysconf(name) \
((__builtin_constant_p (name) \
&& ((name) == _SC_PAGESIZE || (name) == _SC_RE_DUP_MAX)) \
? __glibc_sysconf_const (name) \
: sysconf (name))
#endif
Of course add any other relevant _SC_... values to the disjunct.
* Paul Eggert:
> On 2024-10-01 15:54, Florian Weimer wrote:
>> The tricky part is that we want to tell GCC that sysconf (_SC_PAGE_SIZE)
>> is a constant, but sysconf in general is not __attribute__
>> ((__const__)). The easiest way to implement that is to call
>> __getpagesize, which has the right attribute. The patch doesn't
>> implement that, though.
>
> Can we do something like the following to make sysconf (_SC_PAGESIZE)
> effectively const?
>
> #ifdef __extern_always_inline
> __extern_always_inline __attribute_const__ long int
> __glibc_sysconf_const (int __name)
> {
> return sysconf (__name);
> }
>
> # define sysconf(name) \
> ((__builtin_constant_p (name) \
> && ((name) == _SC_PAGESIZE || (name) == _SC_RE_DUP_MAX)) \
> ? __glibc_sysconf_const (name) \
> : sysconf (name))
> #endif
>
> Of course add any other relevant _SC_... values to the disjunct.
I think we should avoid the macro, which is possible with __REDIRECT_NTH
and a local const alias, I believe. We just need to hope that the
compiler applies the const attribute to the alias, and not the called
function. And we'd probably want to call __getpagesize anyway because
it already exists, so that we don't have to go through that switch
statement in the sysconf implementation.
Thanks,
Florian
On Tue, 1 Oct 2024, Paul Eggert wrote:
> > And why is it going to be a problem?
>
> It's in a standard as a constraint, which means standard compilers are
> required to issue a diagnostic, unfortunately.
Thank you, Captain Obvious.
> > The ISO C24 clause you refer says:
>
> Oops, I meant to cite paragraph 3 not paragraph 2. Sorry about that. Paragraph
> 3 says, "An inline definition of a function with external linkage shall not
> ... contain, anywhere in the tokens making up the function definition, a
> reference to an identifier with internal linkage." This means a user-defined
> extern inline function cannot call a static function.
Is it a correct interpretation though that this applies to static inline
function definitions? Those definitions have always been a type-checked
equivalent to macros and to make them cause a compilation warning as from
ISO C24 seems counter-productive (with "always" as in "for 20+ years").
Needless to say GCC doesn't issue any diagnostics for this program:
static inline int
bar (void)
{
return 0;
}
extern inline int
foo (void)
{
return bar ();
}
int
main (void)
{
return foo ();
}
-- not even at `-Wall -W -pedantic -std=iso9899:2024' (and regardless of
optimisation or the lack of requested).
> I suppose we could do something like this:
>
> #if __GNUC_PREREQ (3,2) || __glibc_has_attribute (__always_inline__)
> static __always_inline long int
> __glibc_sysconf (int __name)
> {
> ... optimizations go here ...
> }
> # define sysconf(name) __glibc_sysconf (name)
> #endif
>
> This would support the C standard rules, so long as GCC is smart enough to not
> issue a diagnostic about this sort of thing (is it?).
This is a definition of a static inline function, the calling of which
you just told me is not supported from extern inline functions as from ISO
C24, so I'm even more confused now.
Maciej
On 2024-10-02 02:47, Maciej W. Rozycki wrote:
> Is it a correct interpretation though that this applies to static inline
> function definitions?
I see no other way to interpret it. I agree it's a dumb rule. I don't
know why the rule is there.
>> I suppose we could do something like this:
>>
>> #if __GNUC_PREREQ (3,2) || __glibc_has_attribute (__always_inline__)
>> static __always_inline long int
>> __glibc_sysconf (int __name)
>> {
>> ... optimizations go here ...
>> }
>> # define sysconf(name) __glibc_sysconf (name)
>> #endif
>>
>> This would support the C standard rules, so long as GCC is smart enough to not
>> issue a diagnostic about this sort of thing (is it?).
>
> This is a definition of a static inline function, the calling of which
> you just told me is not supported from extern inline functions as from ISO
> C24, so I'm even more confused now.
This function is defined using syntax that is an extension to C, and so
it does not need to follow all the rules that the standard requires.
The problem I originally referred to was in a user-defined extern inline
function that calls a system-header-defined static function that does
not use C extensions. If that's done with GCC I suppose we can talk the
GCC maintainers into not diagnosing it (which apparently it's already
doing); but if it's done with some other compiler things would quite
possibly not be so smooth.
new file mode 100644
@@ -0,0 +1,10 @@
+#error "This file must be written based on the possible page sizes"
+
+/* 1 << __GLIBC_PAGE_SHIFT_MIN is the minimum possible page size.
+ The least significant __GLIBC_PAGE_SHIFT_MIN bits of pointers
+ return by mmap are guaranteed to be cleared. */
+#define __GLIBC_PAGE_SHIFT_MIN
+
+/* 1 << __GLIBC_PAGE_SHIFT_MAX is the maximum possible page size.
+ On-disk file formats must not require smaller mapping offsets. */
+#define __GLIBC_PAGE_SHIFT_MAX
new file mode 100644
@@ -0,0 +1 @@
+#include <misc/sys/pagesize.h>
new file mode 100644
@@ -0,0 +1,32 @@
+/* Information about supported page sizes.
+ Copyright (C) 2024 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_PAGESIZE_H
+#define _SYS_PAGESIZE_H
+
+#include <bits/pagesize.h>
+
+/* Minimum and maximum possible page sizes, in bytes. */
+#define PAGE_SIZE_MIN (1UL << __GLIBC_PAGE_SHIFT_MIN)
+#define PAGE_SIZE_MAX (1UL << __GLIBC_PAGE_SHIFT_MAX)
+
+/* Base-2 logarithm of the size limits. */
+#define PAGE_SHIFT_MIN __GLIBC_PAGE_SHIFT_MIN
+#define PAGE_SHIFT_MAX __GLIBC_PAGE_SHIFT_MAX
+
+#endif /* _SYS_PAGESIZE_H */