linux-kernel - Re: [PATCH] kernel/sched/core.c: Modified prio

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y/TEcdIDqGjwH+2f@debian.me>
Date:   Tue, 21 Feb 2023 20:17:37 +0700
From:   Bagas Sanjaya <bagasdotme@...il.com>
To:     JaeJoon Jung <rgbi3307@...il.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kernel/sched/core.c: Modified prio_less().

On Tue, Feb 21, 2023 at 05:24:27PM +0900, JaeJoon Jung wrote:
> The sched_class structure is defined to be sorted by pointer size.
> You can see it in the macro definition like this:
> 
> kernel/sched/sched.h
> #define DEFINE_SCHED_CLASS(name)
> const struct sched_class name##_sched_class \
>         __aligned(__alignof__(struct sched_class)) \
>         __section("__" #name "_sched_class")
> 
> include/asm-generic/vmlinux.lds.h
> #define SCHED_DATA \
> STRUCT_ALIGN(); \
> __sched_class_highest = .; \
> *(__stop_sched_class) \
> *(__dl_sched_class) \
> *(__rt_sched_class) \
> *(__fair_sched_class) \
> *(__idle_sched_class) \
> __sched_class_lowest = .;
> 
> And in the System.map file,
> you can see that they are arranged in memory address order.
> 
> System.map
> ----------------------------------------------------------------
> ffffffff8260d520 R __sched_class_highest
> ffffffff8260d520 R stop_sched_class
> ffffffff8260d5f0 R dl_sched_class
> ffffffff8260d6c0 R rt_sched_class
> ffffffff8260d790 R fair_sched_class
> ffffffff8260d860 R idle_sched_class
> ffffffff8260d930 R __sched_class_lowest
> ----------------------------------------------------------------
> 
> This matches the sched class priority.
> In the prio_less() function in kernel/sched/core.c,
> the less value can be determined by pointer operation as follows.
> 
> If the prio_less() function is modified as follows,
> the __task_prio() function is not required.

By what?

> 
> Thanks,
> 
> 
> Signed-off-by: JaeJoon Jung <rgbi3307@...il.com>
> ---
>  kernel/sched/core.c | 42 +++++++++++-------------------------------
>  1 file changed, 11 insertions(+), 31 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 2a4918a1faa9..75075d92a198 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -151,21 +151,6 @@ __read_mostly int scheduler_running;
> 
>  DEFINE_STATIC_KEY_FALSE(__sched_core_enabled);
> 
> -/* kernel prio, less is more */
> -static inline int __task_prio(struct task_struct *p)
> -{
> - if (p->sched_class == &stop_sched_class) /* trumps deadline */
> - return -2;
> -
> - if (rt_prio(p->prio)) /* includes deadline */
> - return p->prio; /* [-1, 99] */
> -
> - if (p->sched_class == &idle_sched_class)
> - return MAX_RT_PRIO + NICE_WIDTH; /* 140 */
> -
> - return MAX_RT_PRIO + MAX_NICE; /* 120, squash fair */
> -}
> -
>  /*
>   * l(a,b)
>   * le(a,b) := !l(b,a)
> @@ -176,22 +161,17 @@ static inline int __task_prio(struct task_struct *p)
>  /* real prio, less is less */
>  static inline bool prio_less(struct task_struct *a, struct
> task_struct *b, bool in_fi)
>  {
> -
> - int pa = __task_prio(a), pb = __task_prio(b);
> -
> - if (-pa < -pb)
> - return true;
> -
> - if (-pb < -pa)
> - return false;
> -
> - if (pa == -1) /* dl_prio() doesn't work because of stop_class above */
> - return !dl_time_before(a->dl.deadline, b->dl.deadline);
> -
> - if (pa == MAX_RT_PRIO + MAX_NICE) /* fair */
> - return cfs_prio_less(a, b, in_fi);
> -
> - return false;
> +        int less = a->sched_class - b->sched_class;
> +        if (less == 0) {
> +                if (a->sched_class == &dl_sched_class)
> +                        return !dl_time_before(a->dl.deadline, b->dl.deadline);
> +
> +                else if (a->sched_class == &fair_sched_class)
> +                        return cfs_prio_less(a, b, in_fi);
> +                else
> +                        return false;
> +        } else
> +                return (less > 0) ? true : false;
>  }

I smell indentation-corrupted patch here. Please use git-send-email(1)
to submit patches.

For the patch subject, I can't imagine what are you doing since you
wrote too generic subject ("Modified foo") without clearly describe in
the patch description what are you doing. How can maintainers accept
your patch if you don't take care of how to describe it?

Last but not least, don't top-post when replying; reply inline with
appropriate context instead.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)