Il 27/09/2013 15:13, Aneesh Kumar K.V ha scritto:
> Alexander Graf <agraf@xxxxxxx> writes:
>
>> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
>>
>>> From: "Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
>>
>> Missing patch description.
>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
>>
>> I fail to see how this really simplifies things, but at the end of the
>> day it's Gleb's and Paolo's call.
>
> will do. It avoid calling
>
> for_each_online_cpu(cpu) {
> smp_call_function_single()
>
> on multiple architecture.
I agree with Alex.
The current code is not specially awesome; having
kvm_arch_check_processor_compat take an int* disguised as a void* is a
bit ugly indeed.
However, the API makes sense and tells you that it is being passed as a
callback (to smp_call_function_single in this case).
You are making the API more complicated to use on the arch layer
(because arch maintainers now have to think "do I need to check this on
all online CPUs?") and making the "leaf" POWER code less legible because
it still has the weird void()(void *) calling convention.
If anything, you could change kvm_arch_check_processor_compat to return
an int and accept no argument, and introduce a wrapper that kvm_init
passes to smp_call_function_single.
Paolo
> We also want to make the smp call function a callback of opaque. Hence
> this should be made arch specific.
>
> int kvm_arch_check_processor_compat(void *opaque)
> {
> int r,cpu;
> struct kvmppc_ops *kvm_ops = (struct kvmppc_ops *)opaque;
> for_each_online_cpu(cpu) {
> smp_call_function_single(cpu,
> kvm_ops->check_processor_compat,
> &r, 1);
> if (r < 0)
> break;
> }
> return r;
> }
>
> against
>
> - for_each_online_cpu(cpu) {
> - smp_call_function_single(cpu,
> - kvm_arch_check_processor_compat,
> - &r, 1);
> - if (r < 0)
> - goto out_free_1;
> - }
> +
> + r = kvm_arch_check_processor_compat(opaque);
> + if (r < 0)
> + goto out_free_1;
>
>
>
>>
>> Which brings me to the next issue: You forgot to CC kvm@vger on your
>> patch set. Gleb and Paolo don't read kvm-ppc@vger. And they shouldn't
>> have to. Every kvm patch that you want review on or that should get
>> applied needs to be sent to kvm@vger. If you want to tag it as PPC
>> specific patch, do so by CC'ing kvm-ppc@vger.
>
> Will do in the next update
>
> -aneesh
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
