Re: [Minios-devel] [UNIKRAFT PATCH v3 08/12] lib/uksched: Set scheduler reference in scheduler implementation

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

Hi Costin,

I wonder whether for this functionality, it would make more sense to 
just change sched<whateverimplementation>_thread_add to return an int as 
a success/error code and then let uk_sched_thread_add react to that? I 
see two advantages: One, it lets the scheduler framework deal with the 
fallout from the errors of the implementation and thus reduce potential 
code duplication: it could then destroy and clean up the thread. Or it 
could even try out a second implementation if we have some crazy 
multi-scheduler setups! Two, it would mean that someone writing a 
concrete scheduler implementation doesn't have to remember to set 
t->sched, but only deal with the more standard "0 on success, -1 on 
error" return logic.
Cheers,
Florian


On 3/10/19 9:10 PM, Costin Lupu wrote:
> Scheduler reference should be set in scheduler 'methods' because this is
> done after validations specific to scheduler implementation are made.
> Thus, an error might prevent threads of being added/removed from
> schedulers internal collections.
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> ---
>   lib/uksched/include/uk/sched.h | 2 --
>   lib/uksched/thread.c           | 1 +
>   lib/ukschedcoop/schedcoop.c    | 1 +
>   3 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/uksched/include/uk/sched.h b/lib/uksched/include/uk/sched.h
> index 9e218688..0e8f1e26 100644
> --- a/lib/uksched/include/uk/sched.h
> +++ b/lib/uksched/include/uk/sched.h
> @@ -114,7 +114,6 @@ static inline void uk_sched_thread_add(struct uk_sched *s,
>         UK_ASSERT(t);
>         if (attr)
>                 t->detached = attr->detached;
> -       t->sched = s;
>         s->thread_add(s, t, attr);
>   }
>   
> @@ -124,7 +123,6 @@ static inline void uk_sched_thread_remove(struct uk_sched *s,
>         UK_ASSERT(s);
>         UK_ASSERT(t);
>         s->thread_remove(s, t);
> -       t->sched = NULL;
>   }
>   
>   static inline int uk_sched_thread_set_prio(struct uk_sched *s,
> diff --git a/lib/uksched/thread.c b/lib/uksched/thread.c
> index 8602402a..e52bb30f 100644
> --- a/lib/uksched/thread.c
> +++ b/lib/uksched/thread.c
> @@ -89,6 +89,7 @@ int uk_thread_init(struct uk_thread *thread,
>         thread->wakeup_time = 0LL;
>         thread->detached = false;
>         uk_waitq_init(&thread->waiting_threads);
> +       thread->sched = NULL;
>   
>   #ifdef CONFIG_HAVE_LIBC
>         //TODO _REENT_INIT_PTR(&thread->reent);
> diff --git a/lib/ukschedcoop/schedcoop.c b/lib/ukschedcoop/schedcoop.c
> index a3f265c6..c19a276a 100644
> --- a/lib/ukschedcoop/schedcoop.c
> +++ b/lib/ukschedcoop/schedcoop.c
> @@ -139,6 +139,7 @@ static void schedcoop_thread_add(struct uk_sched *s, struct 
> uk_thread *t,
>         struct schedcoop_private *prv = s->prv;
>   
>   	set_runnable(t);
> +       t->sched = s;
>   
>   	flags = ukplat_lcpu_save_irqf();
>         UK_TAILQ_INSERT_TAIL(&prv->thread_list, t, thread_list);
--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel