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

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Tue, 2017-09-26 at 09:14 -0600, Jan Beulich wrote:
> > > > On 15.09.17 at 20:01, <dario.faggioli@xxxxxxxxxx> wrote:
> > --- a/xen/common/rcupdate.c
> > +++ b/xen/common/rcupdate.c
> > +    int ret = 0;
> > +
> > +    if ( MILLISECS(period) > IDLE_TIMER_PERIOD_MAX )
> > +    {
> > +        printk("WARNING: rcu_idle_timer_period_ms must be <
> > %"PRI_stime"\n",
> > +               IDLE_TIMER_PERIOD_MAX / MILLISECS(1));
> > +        ret = -EINVAL;
> > +    }
> > +    else
> > +        idle_timer_period = MILLISECS(period);
> > +
> > +    printk("RCU idle timer period: %lums\n", period);
> > +
> > +    return ret;
> > +}
> > +custom_param("rcu_idle_timer_period_ms", parse_idle_timer_period);
> 
> Does this really need handling as custom_param(). I.e. wouldn't
> integer_param() plus sanitizing in rcu_init() suffice? 
>
I did it like that in the first place. But then I did not like the
overall look of the patch, so I changed the approach.

I can put it back together the integer_param() variant, and you'll tell
me which one you like better.

> That would
> also make the log message be printed uniformly - merely echoing
> the value from the command line doesn't look very useful to me.
>
Mmm.. Sorry, but I don't get this part.

> Additionally - what about a lower bound? Clearly zero would be
> a rather bad choice?
> 
Indeed I should enforce a meaningful min too (I'll take it from patch 3
and put it here).

> Also can you please switch to - as the separator in the command
> line argument name?
> 
Ah, ok.

Thanks and 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