Re: [Xen-devel] Question about apic ipi interface

[Stefan Bader <stefan.bader@xxxxxxxxxxxxx>]

On 28.05.2013 14:43, Konrad Rzeszutek Wilk wrote:
> On Thu, May 23, 2013 at 11:24:56AM +0200, Stefan Bader wrote:
>> On 22.05.2013 21:40, Konrad Rzeszutek Wilk wrote:
>>> On Wed, May 08, 2013 at 06:26:40PM +0200, Stefan Bader wrote:
>>>> On 23.04.2013 12:07, Stefan Bader wrote:
>>>>> I was looking at some older patch and there is one thing I do not 
>>>>> understand.
>>>>>
>>>>> commit f447d56d36af18c5104ff29dcb1327c0c0ac3634
>>>>>     xen: implement apic ipi interface
>>>>>
>>>>> Specifically there the implementation of xen_send_IPI_mask_allbutself().
>>>>>
>>>>> void xen_send_IPI_mask_allbutself(const struct cpumask *mask,
>>>>>                                 int vector)
>>>>> {
>>>>>         unsigned cpu;
>>>>>         unsigned int this_cpu = smp_processor_id();
>>>>>
>>>>>         if (!(num_online_cpus() > 1))
>>>>>                 return;
>>>>>
>>>>>         for_each_cpu_and(cpu, mask, cpu_online_mask) {
>>>>>                 if (this_cpu == cpu)
>>>>>                         continue;
>>>>>
>>>>>                 xen_smp_send_call_function_single_ipi(cpu);
>>>>>         }
>>>>> }
>>>>>
>>>>> Why is this using xen_smp_send_call_function_single_ipi()? This dumps the
>>>>> supplied vector and always uses XEN_CALL_FUNCTION_SINGLE_VECTOR. In 
>>>>> contrast the
>>>>> xen_send_IPI_all() and xen_send_IPI_self() keep the (mapped) vector.
>>>>>
>>>>> Mildly wondering about whether call function would need special casing 
>>>>> (just
>>>>> because xen_smp_send_call_function_ipi() is special). But I don't have 
>>>>> the big
>>>>> picture there.
>>>>>
>>>>
>>>> This never got really answered, so lets try this: Does the following patch 
>>>> seem
>>>> to make sense? I know, it has not caused any obvious regressions but at 
>>>> least
>>>> this would look more in agreement with the other code. It has not blown up 
>>>> on a
>>>> normal boot either.
>>>> Ben, is there a simple way that I would trigger the problem you had?
>>>>
>>>> -Stefan
>>>>
>>>>
>>>> From e13703426f367c618f2984d376289b197a8c0402 Mon Sep 17 00:00:00 2001
>>>> From: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
>>>> Date: Wed, 8 May 2013 16:37:35 +0200
>>>> Subject: [PATCH] xen: Clean up apic ipi interface
>>>>
>>>> Commit f447d56d36af18c5104ff29dcb1327c0c0ac3634 introduced the
>>>> implementation of the PV apic ipi interface. But there were some
>>>> odd things (it seems none of which cause really any issue but
>>>> maybe they should be cleaned up anyway):
>>>>  - xen_send_IPI_mask_allbutself (and by that xen_send_IPI_allbutself)
>>>>    ignore the passed in vector and only use the CALL_FUNCTION_SINGLE
>>>>    vector. While xen_send_IPI_all and xen_send_IPI_mask use the vector.
>>>>  - physflat_send_IPI_allbutself is declared unnecessarily. It is never
>>>>    used.
>>>>
>>>> This patch tries to clean up those things.
>>>>
>>>> Signed-off-by: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
>>>
>>> Looks very similar to 
>>>
>>> https://patchwork.kernel.org/patch/2414311/
>>>
>>> So two people pointing out the same thing. 
>>
>> Yeah, from this discussion and further looking into it I am relatively sure 
>> this
>> has no visible effect either way because there currently is no user of the 
>> "odd"
>> implementations.
> 
> OK, could you resend your patch properly please? Somehow I cannot apply
> your patch:
> 
> patching file arch/x86/xen/smp.c
> patch: **** malformed patch at line 136: ask *mask,
> 

Thunderbird joy... as attachment this time...

-Stefan

Attachment: 0001-xen-Clean-up-apic-ipi-interface.patch
Description: Text Data

Attachment: signature.asc
Description: OpenPGP digital signature

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