From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ross Lagerwall <ross.lagerwall@citrix.com>
Subject: Re: [PATCH v3 07/23] xsplice: Implement support for
 applying/reverting/replacing patches. (v5)
Date: Mon, 22 Feb 2016 15:00:31 +0000
Message-ID: <56CB228F.2090101@citrix.com>
References: <1455300361-13092-1-git-send-email-konrad.wilk@oracle.com>
	<1455300361-13092-8-git-send-email-konrad.wilk@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1455300361-13092-8-git-send-email-konrad.wilk@oracle.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, xen-devel@lists.xenproject.org, andrew.cooper3@citrix.com, konrad@kernel.org, mpohlack@amazon.de, sasha.levin@citrix.com, jinsong.liu@alibaba-inc.com, Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, Keir Fraser <keir@xen.org>, Jan Beulich <jbeulich@suse.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>, Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>, Jun Nakajima <jun.nakajima@intel.com>, Kevin Tian <kevin.tian@intel.com>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

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.

-- 
Ross Lagerwall