Re: [Xen-devel] [PATCH v3 07/23] xsplice: Implement support for applying/reverting/replacing patches. (v5)

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Mon, Feb 22, 2016 at 03:00:31PM +0000, Ross Lagerwall wrote:
> On 02/12/2016 06:05 PM, Konrad Rzeszutek Wilk wrote:
> snip
> >+static void xsplice_do_single(unsigned int total_cpus)
> >+{
> >+    nmi_callback_t saved_nmi_callback;
> >+    struct payload *data, *tmp;
> >+    s_time_t timeout;
> >+    int rc;
> >+
> >+    data = xsplice_work.data;
> >+    timeout = xsplice_work.timeout + NOW();
> >+    if ( xsplice_do_wait(&xsplice_work.semaphore, timeout, total_cpus,
> >+                         "Timed out on CPU semaphore") )
> >+        return;
> >+
> >+    /* "Mask" NMIs. */
> >+    saved_nmi_callback = set_nmi_callback(mask_nmi_callback);
> >+
> >+    /* All CPUs are waiting, now signal to disable IRQs. */
> >+    xsplice_work.ready = 1;
> >+    smp_wmb();
> >+
> >+    atomic_inc(&xsplice_work.irq_semaphore);
> >+    if ( xsplice_do_wait(&xsplice_work.irq_semaphore, timeout, total_cpus,
> >+                         "Timed out on IRQ semaphore.") )
> >+        return;
> >+
> >+    local_irq_disable();
> >+    /* Now this function should be the only one on any stack.
> >+     * No need to lock the payload list or applied list. */
> >+    switch ( xsplice_work.cmd )
> >+    {
> >+    case XSPLICE_ACTION_APPLY:
> >+        rc = apply_payload(data);
> >+        if ( rc == 0 )
> >+            data->state = XSPLICE_STATE_APPLIED;
> >+        break;
> >+    case XSPLICE_ACTION_REVERT:
> >+        rc = revert_payload(data);
> >+        if ( rc == 0 )
> >+            data->state = XSPLICE_STATE_CHECKED;
> >+        break;
> >+    case XSPLICE_ACTION_REPLACE:
> >+        list_for_each_entry_safe_reverse ( data, tmp, &applied_list, list )
> >+        {
> >+            data->rc = revert_payload(data);
> >+            if ( data->rc == 0 )
> >+                data->state = XSPLICE_STATE_CHECKED;
> >+            else
> >+            {
> >+                rc = -EINVAL;
> >+                break;
> >+            }
> >+        }
> 
> You're using data as a loop iterator here but the variable serves another
> purpose outside the loop. That's not gonna end well.

No not at all. I've added another variable: "other" that will be used in the 
loop.

> 
> -- 
> Ross Lagerwall

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