Re: [PATCH v6 09/24] xsplice: Implement payload loading

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Apr 11, 2016 at 02:21:55PM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Apr 11, 2016 at 11:26:05AM -0600, Jan Beulich wrote:
> > >>> On 11.04.16 at 19:08, <konrad.wilk@oracle.com> wrote:
> > > If the system admin continously tried to unload and load the patchset
> > > then we certainly would spam.
> > > 
> > > But the 'loading' is (or ought to) be a single event. The applying
> > > or reverting may be done more often.
> > > 
> > > As such I would say that the operations that are tied to apply/reverting
> > > should go through printk - to at least leave breadcrumbs if things
> > > fall apart. I would say:
> > > 
> > >         printk(XENLOG_ERR XSPLICE "%s: unable to get cpu_maps lock!\n",
> > >         printk(XENLOG_ERR XSPLICE "%s: Timed out on %s semaphore %u/%u\n",
> > >             printk(XENLOG_ERR XSPLICE "%s: CPU%u - unable to get cpu_maps lock!\n",
> > >         printk(XENLOG_INFO XSPLICE "%s finished %s with rc=%d\n",
> > >         printk(XENLOG_INFO XSPLICE ": build-id: %*phN\n", len, binary_id);
> > >     dprintk(XENLOG_DEBUG, XSPLICE "%s: Applying %u functions.\n",
> > >     dprintk(XENLOG_DEBUG, XSPLICE "%s: Reverting.\n", data->name);
> > >     dprintk(XENLOG_DEBUG, XSPLICE "%s: timeout is %"PRI_stime"ms\n",
> > >             dprintk(XENLOG_DEBUG, XSPLICE "%s: CPU%u - IPIing the other %u CPUs\n",
> > > 
> > > Should be come printk. And make them INFO (except on errors - they should be 
> > > ERR).
> > 
> > Especially for the last one I don't see what use this has outside of
> > debugging activities. For the others a primary question is: Can any
> 
> True, last one is very much debug.
> 
> > of these occur more than once for a single operation (hypercall)?
> 
> The apply/replace hypercall can 
> 
>      dprintk(XENLOG_DEBUG, XSPLICE "%s: timeout is %"PRI_stime"ms\n",
>      dprintk(XENLOG_DEBUG, XSPLICE "%s: Applying %u functions.\n",
>          printk(XENLOG_INFO XSPLICE "%s finished %s with rc=%d\n",
> 
> The replace can trigger a lot of "Reverting".. And one "Applying"
> 
> The uploading can trigger tons of them if payload is buggy.
> 
> The 'get' and 'list' are silent.

With that in mind the series only has these be printk:

[konrad@char xen]$ git diff origin/staging.. | grep printk | grep -v dprintk | grep XEN > /tmp/z

+    printk(XENLOG_INFO XSPLICE "%s: Applying %u functions.\n",
+    printk(XENLOG_INFO XSPLICE "%s: Reverting.\n", data->name);
+        printk(XENLOG_ERR XSPLICE "%s: unable to get cpu_maps lock!\n",
+        printk(XENLOG_ERR XSPLICE "%s: Timed out on %s semaphore %u/%u\n",
+            printk(XENLOG_ERR XSPLICE "%s: CPU%u - unable to get cpu_maps lock!\n",
+        printk(XENLOG_INFO XSPLICE "%s finished %s with rc=%d\n",
+        printk(XENLOG_INFO XSPLICE ": build-id: %*phN\n", len, binary_id);

while the rest are in dprintk with XENLOG_DEBUG (10), XENLOG_ERR (47) levels.

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