Re: [Xen-devel] [PATCH 13/19] xen: credit2: make the code less experimental

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Mon, 2016-06-20 at 02:13 -0600, Jan Beulich wrote:
> > > > On 18.06.16 at 01:12, <dario.faggioli@xxxxxxxxxx> wrote:
> > @@ -680,8 +677,8 @@ __update_svc_load(const struct scheduler *ops,
> >          delta = now - svc->load_last_update;
> >          if ( unlikely(delta < 0) )
> >          {
> > -            d2printk("%s: Time went backwards? now %"PRI_stime"
> > llu %"PRI_stime"\n",
> > -                     __func__, now, svc->load_last_update);
> > +            printk("WARNING: %s: Time went backwards? now
> > %"PRI_stime" llu %"PRI_stime"\n",
> > +                   __func__, now, svc->load_last_update);
> >              delta = 0;
> >          }
> >  
> With these now becoming non-debugging ones - are they useful
> _every_ time such an event occurs? I.e. wouldn't it be better to
> e.g. only log new high watermark values?
>
Actually, I may want to reconsider this specific hunk (and the other
similar ones using printk instead of d2printk for 'time going
backwards' debug lines).

It's useful, but I'm not sure I want it printing all the time.

So hold off committing this patch, please (this is probably not
necessary to say, given the issue below, but just in case...)

> > @@ -2580,15 +2583,20 @@ csched2_init(struct scheduler *ops)
> >      int i;
> >      struct csched2_private *prv;
> >  
> > -    printk("Initializing Credit2 scheduler\n" \
> > -           " WARNING: This is experimental software in
> > development.\n" \
> > +    printk("Initializing Credit2 scheduler\n");
> > +    printk(" WARNING: This is experimental software in
> > development.\n" \
> >             " Use at your own risk.\n");
> >  
> > -    printk(" load_precision_shift: %d\n",
> > opt_load_precision_shift);
> > -    printk(" load_window_shift: %d\n", opt_load_window_shift);
> > -    printk(" underload_balance_tolerance: %d\n",
> > opt_underload_balance_tolerance);
> > -    printk(" overload_balance_tolerance: %d\n",
> > opt_overload_balance_tolerance);
> > -    printk(" runqueues arrangement: %s\n",
> > opt_runqueue_str[opt_runqueue]);
> > +    printk(XENLOG_INFO " load_precision_shift: %d\n"
> > +           " load_window_shift: %d\n"
> > +           " underload_balance_tolerance: %d\n"
> > +           " overload_balance_tolerance: %d\n"
> > +           " runqueues arrangement: %s\n",
> > +           opt_load_precision_shift,
> > +           opt_load_window_shift,
> > +           opt_underload_balance_tolerance,
> > +           opt_overload_balance_tolerance,
> > +           opt_runqueue_str[opt_runqueue]);
> Note that this results in only the first line getting logged at info
> level;
> all others will get the default logging level (i.e. warning)
> assigned. IOW
> I think you want to repeat XENLOG_INFO a couple of times.
> 
You know what, I did not notice that, sorry! Yes, I'll fix this.

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