On Nov 25 21:20, Christian Franke wrote:
> Hi Corinna,
>
> Corinna Vinschen wrote:
> > Hi Christian,
> >
> > On Nov 25 15:00, Christian Franke wrote:
> > > Corinna Vinschen wrote:
> > > > Fixes: ...?
> > > ... the very first commit (cgf 2001) of sched.cc :-)
> > >
> > > New patch attached.
> > >
> > > From e95fc1aceb5287f9ad65c6c078125fecba6c6de9 Mon Sep 17 00:00:00 2001
> > > From: Christian Franke <christian.fra...@t-online.de>
> > > Date: Mon, 25 Nov 2024 14:51:04 +0100
> > > Subject: [PATCH] Cygwin: sched_setscheduler: allow changes of the priority
> > >
> > > Behave like sched_setparam() if the requested policy is identical
> > > to the fixed value (SCHED_FIFO) returned by sched_getscheduler().
> > >
> > > Fixes: 6b2a2aa4af1e ("Add missing files.")
> > Huh, yeah, this is spot on. I wonder if it would make sense to change
> > that to 9a08b2c02eea ("* sched.cc: New file. Implement sched*.")
> > though, given that was the patch intended to add sched.cc :)))
> >
> > Sorry, but I have to ask two more questions:
> >
> > - Isn't returning SCHED_FIFO sched_getscheduler() just as wrong?
>
> Definitly. SCHED_FIFO is a non-preemptive(!) real-time policy. Windows does
> not offer anything like that to userland (AFAIK).
>
> https://man7.org/linux/man-pages/man7/sched.7.html
>
> I wonder whether there was a use case for this emulation when this module
> was introduced in 2001.
Just guessing here, but using one of the RT schedulers was the only way
to enable changing the priority from user space and using SCHED_FIFO was
maybe in error.
> > Shouldn't that be SCHED_OTHER, and sched_setscheduler() should check
> > for that instead? Cygwin in a real-time scenario sounds a bit
> > far-fetched...
>
> Agree.
>
> Note that SCHED_OTHER requires sched_priority == 0, so most of the
> sched_get/set*() priority related code would no longer make sense then.
This is the other problem. Changing this to SCHED_OTHER sounds like
dropping potentially used functionality. Maybe we should just switch to
SCHED_RR?
> A related interesting snippet which I don't understand:
> sys/sched.h:
> #if defined(__CYGWIN__)
> #define SCHED_OTHER 3
> #else
> #define SCHED_OTHER 0
> #endif
Oh, that's for backward compat. The original sched.h in Cygwin defined
SCHED_OTHER as 3, while newlib's sys/sched.h defined SCHED_OTHER as 0.
The right thing to do in 2001 would have been to define SCHED_OTHER to 0
for Cygwin as well, but unfortunately nobody really cared at the time.
Corinna
