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

From: Dario Faggioli <dario.faggioli@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel@lists.xenproject.org,
	Anshul Makkar <anshul.makkar@citrix.com>,
	David Vrabel <david.vrabel@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>
Subject: Re: [PATCH 13/19] xen: credit2: make the code less experimental
Date: Thu, 7 Jul 2016 12:59:25 +0200	[thread overview]
Message-ID: <1467889165.23985.68.camel@citrix.com> (raw)
In-Reply-To: <5767C1D802000078000F6812@prv-mh.provo.novell.com>

[-- Attachment #1.1: Type: text/plain, Size: 3458 bytes --]

On Mon, 2016-06-20 at 02:13 -0600, Jan Beulich wrote:
> > > > On 18.06.16 at 01:12, <dario.faggioli@citrix.com> 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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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