Re: [UNIKRAFT PATCH] include: Introduce uk/preemmpt.h

[Costin Lupu <costin.lupu@xxxxxxxxx>]

Hi Simon,

Please see inline.

On 7/31/20 7:06 PM, Simon Kuenzer wrote:
> Hi Costin,
> 
> fair enough. It makes sense what you say. With this in mind, I propose
> the following improvement for this header:
> 
> We distinguish with CONFIG_HAVE_SCHED if we need to do something
> regarding scheduling or not. This option indicates if the platform
> library is compiled with context switch capabilities.
> 
> For !CONFIG_HAVE_SCHED:
>     We use only memory barriers.
> 

I would be more specific here and I would use `!CONFIG_HAVE_SCHED_PREEMPT`.

> For CONFIG_HAVE_SCHED:
>     We do another check for CONFIG_LIBUKSCHED. If uksched is not there,
> we use interrupts on/off + memory barriers. 

I repeat, we do not disable interrupts here, it has nothing to do with
disabling interrupts. We disable only preemption. As I said in the
previous email, there might be cases (and actually those can be quite
common) where we would encounter deadlocks if we had disabled interrupts.

Therefore it's enough to use the memory barriers, just as it is in the
upstreamed code.

> For the case uksched is there, I expect that an API call in uksched
> is provided to do this nesting count and disable preemption (this
> will probably be mapped to a no-op for the cooperative scheduler and
> an actrual function to preemptive schedulers). The memory barrier
> should stay within the preempt.h header.

If we use `!CONFIG_HAVE_SCHED_PREEMPT` then there is no need to deal
with the uksched + schedcoop case.

> 
> Of course a brief description should be in the header that says that
> this is not about irqs on/off but disabling thread context migration to
> different CPU cores and thread interruption.
> 
> We can put the check for CONFIG_LIBUKSCHED as a TODO for now, because I
> think preempted scheduling needs to be upstreamed first. So I would go
> for irq on/off for now. Later we will implement the uksched case properly.
> 

Again, we don't deal with IRQs here.

> All-in-all, I see this as a kind of a compromise, but we would cover all
> cases including a good enough support for non-uksched scheduling with
> irqs on/off (which may happen for robotics use cases).

Again, we don't deal with IRQs here.

There is no need to complicate this header now given that we should
(hopefully) upstream the preemptive scheduling patches soon and we will
enhance it then.

Cheers,
Costin