Message ID | 2f833cc18d427bf1db6c2653f9ff43a69a5baa70.1727624528.git.fweimer@redhat.com |
---|---|
State | New |
Headers | show |
Series | Teach glibc about possible page sizes and handle gaps in ld.so | expand |
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.
diff --git a/bits/pagesize.h b/bits/pagesize.h new file mode 100644 index 0000000000..d83561d0c1 --- /dev/null +++ b/bits/pagesize.h @@ -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 diff --git a/include/sys/pagesize.h b/include/sys/pagesize.h new file mode 100644 index 0000000000..2533ee7e90 --- /dev/null +++ b/include/sys/pagesize.h @@ -0,0 +1 @@ +#include <misc/sys/pagesize.h> diff --git a/misc/sys/pagesize.h b/misc/sys/pagesize.h new file mode 100644 index 0000000000..b54b6e6396 --- /dev/null +++ b/misc/sys/pagesize.h @@ -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 */