[PATCH] Cygwin: sched_setscheduler: allow changes of the priority

[Corinna Vinschen]

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.franke@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