kern/129164: Wrong priority value for normal processes

[Bruce Evans]

On Tue, 25 Nov 2008, Garrett Cooper wrote:

> On Nov 25, 2008, at 12:31 AM, Unga wrote:
> > FreeBSD grey.lan 7.0-STABLE FreeBSD 7.0-STABLE Sun May 25 2008 i386
> >> Description:
> > The priority value for root and other normal processes is 65504
> > (rtp.prio) where zero (0) is expected.

This value is unexpected, but rtp.prio is meaningless for normal
processes -- the priority (nice value) is given by getpriority(2), not
by rtprio(2).  rtp.prio gives the realtime or idletime priority for
realtime and idletime processes, respectively.  It is meaningless for
normal processes, but rtprio(RTP_LOOKUP, ...) bogusly succeeds and is
supposed to accidentally return 0 for normal processes.  It should either
fail or return a special value for normal processes.  Note that
rtprio(RTP_SET,. ...) provides no way to manage normal processes, not
even to change back to normal.

> > I checked the program flow from /usr/src/usr.bin/su/su.c to /usr/src/
> > lib/libutil/login_class.c and it looks setusercontext() is setting
> > the priority zero (0) right but the moment it come out from the
> > setusercontext() call in su.c, the priority has already turn to 65504.

login_class only sets normal priorities (nice values).

> > I have marked this issue as "serious". It is serious because normal
> > priority processes crawl on my machine.

Apparently, another bug messes up handling of normal priorities and gives
different garbage in rtp.prio for normal processes.

> > Maximum priority value for normal priority processes can take is 20,
> > not 65504. Normal priority processes are expected to run at priority
> > zero (0) as it is specified in /etc/login.conf under login class
> > "default".

For normal priorities, the range is [-20, 20].

For rtp.prio, the range is [0, 31].

> 	rtp.prio is an unsigned short, and there is some rollover because the
> number set for the priority is then subtracted by 128 (see the
> following grepped snippets). This appears to be a cosmetic issue which
> should be fixed by shifting the starting offset for the priorities.

rtp.prio is supposed to be an integer between 0 and RTPRIO_MAX (31)
inclusive, so the unsigned short type for it is bogus but there should
be no problem with the subtraction -- the biased value is just supposed
to be between PRI_MIN_REALTIME (128) and PRI_MAX_REALTIME (159) inclusive.

65504 is probably -32 misrepresented as an unsigned short.

I can't see any problems with the range being subtracted -- there used to
be problems when td_base_pri was used (since this value became wrong
and possibly out of bounds with priority propagation), but these were
fixed by using td_base_user_pri (in 6.x IIRC -- 7.0 has these fixes).

See top/machine.c for related comments about determining rtp.prio, and
use top(1) or maybe ps(1) to see priorities of all types.  Unfortunately,
the comments are not out of date, and ps is more broken than top, since
fill_kinfo_proc_only() still returns only td_base_pri (as pri_native)
to userland but td_base_user_pri is what is more needed, and ps needs
the right value more than top.

Also, td_base_user_pri seems to be missing initialization for most
cases (ithreads, non-realtime/idletime kthreads, and possibly ordinary
threads with nonzero niceness -- shouldn't the nice bias be included
directly in the base?).  The value default of PUSER seems to be only
changed by rtprio(2).  This might break priority propagation and would
certainly result in userland still not being able to trust pri_native
when pri_native is made td_base_user_pri.

Bruce