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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.09.17 at 20:01, <dario.faggioli@xxxxxxxxxx> wrote:
> --- a/xen/common/rcupdate.c
> +++ b/xen/common/rcupdate.c
> @@ -110,10 +110,35 @@ struct rcu_data {
>   * About how far in the future the timer should be programmed each time,
>   * it's hard to tell (guess!!). Since this mimics Linux's periodic timer
>   * tick, take values used there as an indication. In Linux 2.6.21, tick
> - * period can be 10ms, 4ms, 3.33ms or 1ms. Let's use 10ms, to enable
> - * at least some power saving on the CPU that is going idle.
> + * period can be 10ms, 4ms, 3.33ms or 1ms.
> + *
> + * By default, we use 10ms, to enable at least some power saving on the
> + * CPU that is going idle. The user can change this, via a boot time
> + * parameter, but only up to 100ms.
>   */
> -#define RCU_IDLE_TIMER_PERIOD MILLISECS(10)
> +#define IDLE_TIMER_PERIOD_MAX     MILLISECS(100)
> +#define IDLE_TIMER_PERIOD_DEFAULT MILLISECS(10)
> +static s_time_t __read_mostly idle_timer_period = IDLE_TIMER_PERIOD_DEFAULT;
> +
> +static int parse_idle_timer_period(const char *s)

__init

> +{
> +    long unsigned int period = simple_strtoul(s, NULL, 10);

unsigned long

> +    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? 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.
Or are you concerned about the value being specified in e.g.
hex rather than dec?

Additionally - what about a lower bound? Clearly zero would be
a rather bad choice?

Also can you please switch to - as the separator in the command
line argument name?

Jan


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