Re: [PATCH 2/3] Mini-OS: drop in_callback variable

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Juergen Gross, le lun. 11 déc. 2023 14:48:26 +0100, a ecrit:
> Now that do_hypervisor_callback() is always called with interrupts off
> the in_callback variable meant to handle recursive calls of
> do_hypervisor_callback() can be dropped.
> 
> As a precaution add a test to do_hypervisor_callback() that interrupts
> are really disabled and crash in case this is not true.

That paragraph was for the previous commit ;)

> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>

Thanks!

> ---
>  hypervisor.c         |  6 ------
>  include/hypervisor.h |  2 --
>  sched.c              | 11 -----------
>  3 files changed, 19 deletions(-)
> 
> diff --git a/hypervisor.c b/hypervisor.c
> index 5309daa3..6facce3e 100644
> --- a/hypervisor.c
> +++ b/hypervisor.c
> @@ -37,8 +37,6 @@ EXPORT_SYMBOL(hypercall_page);
>      ((sh)->evtchn_pending[idx] &                \
>       ~(sh)->evtchn_mask[idx])
>  
> -int in_callback;
> -
>  #ifndef CONFIG_PARAVIRT
>  extern shared_info_t shared_info;
>  
> @@ -104,8 +102,6 @@ void do_hypervisor_callback(struct pt_regs *regs)
>  
>      BUG_ON(!irqs_disabled());
>  
> -    in_callback = 1;
> -   
>      vcpu_info->evtchn_upcall_pending = 0;
>      /* NB x86. No need for a barrier here -- XCHG is a barrier on x86. */
>  #if !defined(__i386__) && !defined(__x86_64__)
> @@ -127,8 +123,6 @@ void do_hypervisor_callback(struct pt_regs *regs)
>              do_event(port, regs);
>          }
>      }
> -
> -    in_callback = 0;
>  }
>  
>  void force_evtchn_callback(void)
> diff --git a/include/hypervisor.h b/include/hypervisor.h
> index 1d092719..b852a42a 100644
> --- a/include/hypervisor.h
> +++ b/include/hypervisor.h
> @@ -50,6 +50,4 @@ void mask_evtchn(uint32_t port);
>  void unmask_evtchn(uint32_t port);
>  void clear_evtchn(uint32_t port);
>  
> -extern int in_callback;
> -
>  #endif /* __HYPERVISOR_H__ */
> diff --git a/sched.c b/sched.c
> index e162cb60..3335efa0 100644
> --- a/sched.c
> +++ b/sched.c
> @@ -76,11 +76,6 @@ void schedule(void)
>      prev = current;
>      local_irq_save(flags); 
>  
> -    if (in_callback) {
> -        printk("Must not call schedule() from a callback\n");
> -        BUG();
> -    }
> -
>      do {
>          /* Examine all threads.
>             Find a runnable thread, but also wake up expired ones and find the
> @@ -151,15 +146,12 @@ struct thread* create_thread(char *name, void 
> (*function)(void *), void *data)
>  EXPORT_SYMBOL(create_thread);
>  
>  #ifdef HAVE_LIBC
> -static struct _reent callback_reent;
>  struct _reent *__getreent(void)
>  {
>      struct _reent *_reent;
>  
>      if (!threads_started)
>          _reent = _impure_ptr;
> -    else if (in_callback)
> -        _reent = &callback_reent;
>      else
>          _reent = &get_current()->reent;
>  
> @@ -246,9 +238,6 @@ void init_sched(void)
>  {
>      printk("Initialising scheduler\n");
>  
> -#ifdef HAVE_LIBC
> -    _REENT_INIT_PTR((&callback_reent))
> -#endif
>      idle_thread = create_thread("Idle", idle_thread_fn, NULL);
>  }
>  
> -- 
> 2.35.3
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.