Re: [PATCH] sched_attr: Do not define for glibc >= 2.41

[John Kacur <jkacur@redhat.com>]

On Wed, 29 Jan 2025, Crystal Wood wrote:

> On Wed, 2025-01-29 at 16:54 -0500, John Kacur wrote:
> > 
> > On Tue, 28 Jan 2025, Khem Raj wrote:
> > 
> > > glibc 2.41+ has added [1] definitions for sched_setattr and sched_getattr functions
> > > and struct sched_attr. Therefore, it needs to be checked for here as well before
> > > defining sched_attr
> > > 
> > > Define sched_attr conditionally on SCHED_ATTR_SIZE_VER0
> > > 
> > > Fixes builds with glibc/trunk
> > > 
> > > [1] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=21571ca0d70302909cf72707b2a7736cf12190a0;hp=298bc488fdc047da37482f4003023cb9adef78f8
> > > 
> > > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > > Cc: Clark Williams <williams@redhat.com>
> > > Cc: John Kacur <jkacur@redhat.com>
> > > Cc: rt-users <linux-rt-users@vger.kernel.org>
> > > ---
> > >  src/include/rt-sched.h | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/src/include/rt-sched.h b/src/include/rt-sched.h
> > > index 80171c7..9cf0e3a 100644
> > > --- a/src/include/rt-sched.h
> > > +++ b/src/include/rt-sched.h
> > > @@ -42,6 +42,8 @@
> > >  #define __NR_sched_getattr		275
> > >  #endif
> > >  
> > > +/* sched_attr is not defined in glibc < 2.41 */
> > > +#ifndef SCHED_ATTR_SIZE_VER0
> > >  struct sched_attr {
> > >  	uint32_t size;
> > >  	uint32_t sched_policy;
> > > @@ -68,4 +70,6 @@ int sched_getattr(pid_t pid,
> > >  		  unsigned int size,
> > >  		  unsigned int flags);
> > >  
> > > +#endif /* SCHED_ATTR_SIZE_VER0 */
> > > +
> > >  #endif /* __RT_SCHED_H__ */
> > > 
> > > 
> > 
> > Does the following fix your problem?
> [snip]
> 
> > +#if ! __GLIBC_PREREQ(2, 41)
> 
> I'm not familiar with the norms surrounding use of this macro, but it's
> generally better to test features than versions... what if this feature
> gets backported into some distro's earlier-version-numbered glibc?

The macro is part of the glibc code in features.h
You make a good point about testing the features, but you could also argue 
that there is no guarantee that the implementation will always have 
SCHED_ATTR_SIZE_VER0 and that it could break in the future.

> 
> FWIW, it looks like QEMU went with the feature test approach:
> https://patchwork.ozlabs.org/project/qemu-devel/patch/20241011054806.1014276-1-raj.khem@gmail.com/
> https://patchwork.ozlabs.org/project/qemu-devel/patch/20241011193140.1047648-1-raj.khem@gmail.com/
> 
Well, that's by the same author who sent this patch.


> Also, was that patch posted anywhere?  Just wondering if I've still got
> problems with email filtering.

No, a version of this was sent to me in fedora channels to fix a break in 
rawhide.

The version is more complete than the other patch because it also fixes 
src/lib/rt-sched.c

If this works for folks, lets just use it and move on, it's not worth 
endlessly hashing out.

John Kacur

> 
> -Crystal
> 
> 
>