linux-kernel - Re: [PATCH] sched: Fix rt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20160826044716.GA18417@rhlx01.hs-esslingen.de>
Date:   Fri, 26 Aug 2016 06:47:16 +0200
From:   Andreas Mohr <andi@...as.de>
To:     Yin-goo Yim <ygyim@...kuk.ac.kr>
Cc:     Dongsheng Yang <yangds.fnst@...fujitsu.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched: Fix rt_task to work properly

Hi,

[no properly binding reference via In-Reply-To: available thus manually re-creating, sorry]

> +++ b/include/linux/sched/deadline.h
> @@ -1,13 +1,7 @@
> #ifndef _SCHED_DEADLINE_H
> #define _SCHED_DEADLINE_H
> 
> -/*
> - * SCHED_DEADLINE tasks has negative priorities, reflecting
> - * the fact that any of them has higher prio than RT and
> - * NORMAL/BATCH tasks.
> - */
> -
> -#define MAX_DL_PRIO 0
> +#include <linux/sched/prio.h>
> 
> 
> 
> 
> +++ b/include/linux/sched/deadline.h
> @@ -1,13 +1,7 @@
> #ifndef _SCHED_DEADLINE_H
> #define _SCHED_DEADLINE_H
> 
> -/*
> - * SCHED_DEADLINE tasks has negative priorities, reflecting
> - * the fact that any of them has higher prio than RT and
> - * NORMAL/BATCH tasks.
> - */
> -
> -#define MAX_DL_PRIO 0
> +#include <linux/sched/prio.h>

AFAICS this change is a larger layer violation (dependency issue)
since AFAIK prio.h is a generic, common base header
which is to provide priority definitions
common to *all* of the
more specific scheduler sub handling
(deadline, rt, ...),
which thus should not have to aggregate
any specific knowledge whatsoever about
various scheduler sub type specific handling.

While this "complaint" won't help your case
since it will cause the fix to be semi-incorrect,
some things come to mind:

rt_prio() quite likely is to be seen as an *rt-specific* API
since it is defined in the *rt-specific* rt.h header.

IOW, rt_prio() is *not to be used* for any areas where we
do not have an RT case
(quite certainly header docs should be added to rt_prio()
to definitely emphasize this fact,
maybe something like
"Note that since this is an RT API it is meaningful for RT tasks only").

IOW, it quite likely is some API *user* which is
making a mistake since it uses rt_prio() for cases
where it should be using APIs provided by other non-RT cases.
To be specific, according to your comments
this API user seems to be tg_has_rt_tasks()
which is wrongly calling rt_prio() [not "rt_task", right??]
for tasks where it has NOT yet properly determined
that in fact they *are* RT-type tasks
(the wording of "tg_has_rt_tasks" clearly suggests
that this API does *not* know yet
which task type it currently deals with,
so it does need to figure the type out, reliably,
by *not* directly calling into a fully RT-specific API).

That's at least what I'm inferring from the situation context,
without having looked into details.

BTW, Good Catch!

HTH,

Andreas Mohr