[PATCH] kernel/sched/core.c: Modified prio_less().

[PATCH] kernel/sched/core.c: Modified prio_less().

[JaeJoon Jung]

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.

Thanks,


Signed-off-by: JaeJoon Jung <rgbi3307@gmail.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;
 }

 static inline bool __sched_core_less(struct task_struct *a, struct
task_struct *b)
-- 
2.17.1

Re: [PATCH] kernel/sched/core.c: Modified prio_less().

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 4151 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] kernel/sched/core.c: Modified prio_less().

[JaeJoon Jung]

I'm sorry for the inconvenience caused by my carelessness.
I had the problem you pointed out by copying and sending the contents
generated by git patch as text.
I will learn the git-send-email method you shared and send it again.

Thanks.

On Tue, 21 Feb 2023 at 22:17, Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> 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@gmail.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

Re: [PATCH] kernel/sched/core.c: Modified prio_less().

[Bagas Sanjaya]

On 2/22/23 07:51, JaeJoon Jung wrote:
> I'm sorry for the inconvenience caused by my carelessness.
> I had the problem you pointed out by copying and sending the contents
> generated by git patch as text.
> I will learn the git-send-email method you shared and send it again.
> 

OK but please don't top-post your reply in the future. Doing so, other
people (like me) don't see the context you write (hence I delete
it).

Thanks!

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