Re: [PATCH] Move cpu_down and other functions inside SMP ifdef

[Samuel Thibault]

Applied, thanks!

Flavio Cruz, le mer. 04 janv. 2023 22:45:45 -0500, a ecrit:
> cpu_down is shown as not required when disabling SMP so moving it inside
> NCPUS > 1 to eliminate the warning. Note that the diff ended up looking
> a bit different due the way functions are laid out.
> ---
>  kern/machine.c   | 119 ++++++++++++++++++++++++-----------------------
>  kern/processor.h |   2 -
>  2 files changed, 60 insertions(+), 61 deletions(-)
> 
> diff --git a/kern/machine.c b/kern/machine.c
> index 6f481a5c..5cbf550a 100644
> --- a/kern/machine.c
> +++ b/kern/machine.c
> @@ -98,6 +98,27 @@ void cpu_up(int cpu)
>       pset_unlock(&default_pset);
>  }
>  
> +kern_return_t
> +host_reboot(const host_t host, int options)
> +{
> +     if (host == HOST_NULL)
> +             return (KERN_INVALID_HOST);
> +
> +     if (options & RB_DEBUGGER) {
> +             Debugger("Debugger");
> +     } else {
> +#ifdef parisc
> +/* XXX this could be made common */
> +             halt_all_cpus(options);
> +#else
> +             halt_all_cpus(!(options & RB_HALT));
> +#endif
> +     }
> +     return (KERN_SUCCESS);
> +}
> +
> +#if  NCPUS > 1
> +
>  /*
>   *   cpu_down:
>   *
> @@ -125,26 +146,6 @@ static void cpu_down(int cpu)
>       splx(s);
>  }
>  
> -kern_return_t
> -host_reboot(const host_t host, int options)
> -{
> -     if (host == HOST_NULL)
> -             return (KERN_INVALID_HOST);
> -
> -     if (options & RB_DEBUGGER) {
> -             Debugger("Debugger");
> -     } else {
> -#ifdef parisc
> -/* XXX this could be made common */
> -             halt_all_cpus(options);
> -#else
> -             halt_all_cpus(!(options & RB_HALT));
> -#endif
> -     }
> -     return (KERN_SUCCESS);
> -}
> -
> -#if  NCPUS > 1
>  /*
>   *   processor_request_action - common internals of processor_assign
>   *           and processor_shutdown.  If new_pset is null, this is
> @@ -356,44 +357,6 @@ processor_shutdown(processor_t processor)
>      return(KERN_SUCCESS);
>  }
>  
> -/*
> - *   action_thread() shuts down processors or changes their assignment.
> - */
> -void __attribute__((noreturn)) action_thread_continue(void)
> -{
> -     processor_t     processor;
> -     spl_t           s;
> -
> -     while (TRUE) {
> -             s = splsched();
> -             simple_lock(&action_lock);
> -             while ( !queue_empty(&action_queue)) {
> -                     processor = (processor_t) queue_first(&action_queue);
> -                     queue_remove(&action_queue, processor, processor_t,
> -                                  processor_queue);
> -                     simple_unlock(&action_lock);
> -                     (void) splx(s);
> -
> -                     processor_doaction(processor);
> -
> -                     s = splsched();
> -                     simple_lock(&action_lock);
> -             }
> -
> -             assert_wait((event_t) &action_queue, FALSE);
> -             simple_unlock(&action_lock);
> -             (void) splx(s);
> -             counter(c_action_thread_block++);
> -             thread_block(action_thread_continue);
> -     }
> -}
> -
> -void __attribute__((noreturn)) action_thread(void)
> -{
> -     action_thread_continue();
> -     /*NOTREACHED*/
> -}
> -
>  /*
>   *   processor_doaction actually does the shutdown.  The trick here
>   *   is to schedule ourselves onto a cpu and then save our
> @@ -617,11 +580,48 @@ Restart_pset:
>  
>  }
>  
> +/*
> + *   action_thread() shuts down processors or changes their assignment.
> + */
> +void __attribute__((noreturn)) action_thread_continue(void)
> +{
> +     processor_t     processor;
> +     spl_t           s;
> +
> +     while (TRUE) {
> +             s = splsched();
> +             simple_lock(&action_lock);
> +             while ( !queue_empty(&action_queue)) {
> +                     processor = (processor_t) queue_first(&action_queue);
> +                     queue_remove(&action_queue, processor, processor_t,
> +                                  processor_queue);
> +                     simple_unlock(&action_lock);
> +                     (void) splx(s);
> +
> +                     processor_doaction(processor);
> +
> +                     s = splsched();
> +                     simple_lock(&action_lock);
> +             }
> +
> +             assert_wait((event_t) &action_queue, FALSE);
> +             simple_unlock(&action_lock);
> +             (void) splx(s);
> +             counter(c_action_thread_block++);
> +             thread_block(action_thread_continue);
> +     }
> +}
> +
> +void __attribute__((noreturn)) action_thread(void)
> +{
> +     action_thread_continue();
> +     /*NOTREACHED*/
> +}
> +
>  /*
>   *   Actually do the processor shutdown.  This is called at splsched,
>   *   running on the processor's shutdown stack.
>   */
> -
>  void processor_doshutdown(processor_t processor)
>  {
>       int             cpu = processor->slot_num;
> @@ -645,6 +645,7 @@ void processor_doshutdown(processor_t processor)
>  
>       /*NOTREACHED*/
>  }
> +
>  #else        /* NCPUS > 1 */
>  
>  kern_return_t
> diff --git a/kern/processor.h b/kern/processor.h
> index b81526c0..a099dee8 100644
> --- a/kern/processor.h
> +++ b/kern/processor.h
> @@ -320,8 +320,6 @@ extern kern_return_t processor_set_threads(
>               natural_t       *count);
>  #endif
>  
> -void processor_doaction(processor_t processor);
> -void processor_doshutdown(processor_t processor);
>  void quantum_set(processor_set_t pset);
>  void pset_init(processor_set_t pset);
>  void processor_init(processor_t pr, int slot_num);
> -- 
> 2.37.2
> 
> 

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