George, Dario,
Please ack
On Fri, 28 Jul 2023, Stefano Stabellini wrote:
> On Fri, 28 Jul 2023, Nicola Vetrini wrote:
> > Rule 5.3 has the following headline:
> > "An identifier declared in an inner scope shall not hide an
> > identifier declared in an outer scope"
> >
> > The renaming s/sched_id/scheduler_id/ of the function defined in
> > 'xen/common/sched/core.c' prevents any hiding of that function
> > by the instances of homonymous function parameters that
> > are defined in inner scopes.
> >
> > Similarly, the renames
> > - s/ops/operations/ for the static variable in 'xen/common/sched/core.c'
> > - s/do_softirq/needs_softirq/
> > are introduced for variables, to avoid any conflict with homonymous
> > parameters or function identifiers.
> >
> > Moreover, the variable 'loop' defined at 'xen/common/sched/credit2.c:3887'
> > has been dropped, in favour of the homonymous variable declared in the
> > outer scope. This in turn requires a modification of the printk call that
> > involves it.
> >
> > Signed-off-by: Nicola Vetrini
>
> Reviewed-by: Stefano Stabellini
>
>
> > ---
> > Changes in v3:
> > - removed stray changes to address the remarks
> > Changes in v2:
> > - s/softirq/needs_softirq/
> > - Dropped local variable 'it'
> > - Renamed the 'ops' static variable instead of function parameters
> > in the idle scheduler for coherence.
> >
> > Note: local variable 'j' in xen/common/sched/credit2.c:3812' should
> > probably be unsigned as well, but I saw while editing the patch
> > that it's used as a parameter to 'dump_pcpu', which takes an int.
> > Possibly changing the types of parameters used in these calls is
> > probably a good target for another patch, as it's not relevant
> > w.r.t. Rule 5.3.
> > ---
> > xen/common/sched/core.c| 28 ++--
> > xen/common/sched/credit2.c | 6 +++---
> > xen/common/sysctl.c| 2 +-
> > xen/include/xen/sched.h| 2 +-
> > 4 files changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> > index 022f548652..12deefa745 100644
> > --- a/xen/common/sched/core.c
> > +++ b/xen/common/sched/core.c
> > @@ -91,7 +91,7 @@ extern const struct scheduler
> > *__start_schedulers_array[], *__end_schedulers_arr
> > #define NUM_SCHEDULERS (__end_schedulers_array - __start_schedulers_array)
> > #define schedulers __start_schedulers_array
> >
> > -static struct scheduler __read_mostly ops;
> > +static struct scheduler __read_mostly operations;
> >
> > static bool scheduler_active;
> >
> > @@ -171,7 +171,7 @@ static inline struct scheduler *dom_scheduler(const
> > struct domain *d)
> > * is the default scheduler that has been, choosen at boot.
> > */
> > ASSERT(is_idle_domain(d));
> > -return
> > +return
> > }
> >
> > static inline struct scheduler *unit_scheduler(const struct sched_unit
> > *unit)
> > @@ -2040,10 +2040,10 @@ long do_set_timer_op(s_time_t timeout)
> > return 0;
> > }
> >
> > -/* sched_id - fetch ID of current scheduler */
> > -int sched_id(void)
> > +/* scheduler_id - fetch ID of current scheduler */
> > +int scheduler_id(void)
> > {
> > -return ops.sched_id;
> > +return operations.sched_id;
> > }
> >
> > /* Adjust scheduling parameter for a given domain. */
> > @@ -2579,7 +2579,7 @@ static void cf_check sched_slave(void)
> > struct sched_unit*prev = vprev->sched_unit, *next;
> > s_time_t now;
> > spinlock_t *lock;
> > -bool do_softirq = false;
> > +bool needs_softirq = false;
> > unsigned int cpu = smp_processor_id();
> >
> > ASSERT_NOT_IN_ATOMIC();
> > @@ -2604,7 +2604,7 @@ static void cf_check sched_slave(void)
> > return;
> > }
> >
> > -do_softirq = true;
> > +needs_softirq = true;
> > }
> >
> > if ( !prev->rendezvous_in_cnt )
> > @@ -2614,7 +2614,7 @@ static void cf_check sched_slave(void)
> > rcu_read_unlock(_res_rculock);
> >
> > /* Check for failed forced context switch. */
> > -if ( do_softirq )
> > +if ( needs_softirq )
> > raise_softirq(SCHEDULE_SOFTIRQ);
> >
> > return;
> > @@ -3016,14 +3016,14 @@ void __init scheduler_init(void)
> > BUG_ON(!scheduler);
> > printk("Using '%s' (%s)\n", scheduler->name, scheduler->opt_name);
> > }
> > -ops = *scheduler;
> > +operations = *scheduler;
> >
> > if ( cpu_schedule_up(0) )
> > BUG();
> > register_cpu_notifier(_schedule_nfb);
> >
> > -printk("Using scheduler: %s (%s)\n", ops.name, ops.opt_name);
> > -if ( sched_init() )
> > +printk("Using scheduler: %s (%s)\n", operations.name,
> > operations.opt_name);
> > +if ( sched_init() )
> > panic("scheduler returned error on init\n");
> >
> > if ( sched_ratelimit_us &&
> > @@ -3363,7 +3363,7 @@ int
