Re: [Xen-devel] [PATCH v2 2/3] xen: RCU: make the period of the idle timer configurable.

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Thu, 2017-09-28 at 07:06 -0600, Jan Beulich wrote:
> > > > On 28.09.17 at 12:16, <dario.faggioli@xxxxxxxxxx> wrote:
> > 
> > @@ -569,6 +579,16 @@ void __init rcu_init(void)
> >  {
> >      void *cpu = (void *)(long)smp_processor_id();
> >  
> > +    /* We don't allow 0, or anything higher than
> > IDLE_TIMER_PERIOD_MAX */
> > +    if ( idle_timer_period_ms < 1 ||
> 
> The literal 1 here looks suspicious. How about simply refusing 0
> (as well as too high values)? The also simply document the value
> must be non-zero in the command line doc.
> 
Ok, sure.

> > +         idle_timer_period_ms > IDLE_TIMER_PERIOD_MAX /
> > MILLISECS(1) )
> > +    {
> > +        printk("WARNING: rcu-idle-timer-period-ms outside of
> > [%d,%ld]ms!\n",
> > +               1, IDLE_TIMER_PERIOD_MAX / MILLISECS(1));
> 
> Clearly the %d can be literal 1 if the above literal 1 was to stay.
>
Yes it can. It's actually rather ugly to look at it, the way I managed
to write it... Ewww... sorry! :-P

> If you follow my suggestion, use "(0," instead. 
>
Yeah, I like this too. I was afraid it was a bit too formal, that not
everyone would understand it, but I guess it's actually fine.

> As to the %ld -
> wouldn't that rather need to be PRI_stime (due to MILLISECS()
> returning s_time_t)?
> 
Yes.

> And then, as a cosmetic thing, idle_timer_period_ms now isn't
> really needed outside of this function. I'd prefer if you moved it
> and the integer_param() into this function, to limit their scopes
> as much as possible.
> 
Ok. idle_timer_period_ms still wants to go into __initdata, right?

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel