On Fri, Dec 1, 2017 at 8:18 AM, Andrew Jones <drjones@xxxxxxxxxx> wrote:
> On Fri, Dec 01, 2017 at 09:09:19AM +0100, Christoffer Dall wrote:
>> Hi Drew,
>>
>> On Mon, Nov 27, 2017 at 07:17:18PM +0100, Andrew Jones wrote:
>> > Recently commit b2c9a85dd75a ("KVM: arm/arm64: vgic: Move
>> > kvm_vgic_destroy call around") caught my eye. When I looked closer I
>> > saw that while it made the code saner, it wasn't changing anything.
>> > kvm_for_each_vcpu() checks for NULL kvm->vcpus[i], so there wasn't
>> > a NULL dereference being fixed, and because kvm_vgic_vcpu_destroy()
>> > was called by kvm_arch_vcpu_free() it was still getting called, just
>> > not by kvm_vgic_destroy() as intended. But now the call from
>> > kvm_arch_vcpu_free() is redundant, and while currently harmless, it
>> > should be removed in case kvm_vgic_vcpu_destroy() were ever to
>> > want to reference vgic state, as kvm_vgic_destroy() now comes before
>> > kvm_arch_vcpu_free(). Additionally the other architectures set
>> > kvm->online_vcpus to zero after freeing them. We might as well do
>> > that for ARM too.
>>
>> Could this commit message be rewritten to:
>>
>> kvm_vgic_vcpu_destroy already gets called from kvm_vgic_destroy for
>> each vcpu, so we don't have to call it from kvm_arch_vcpu_free.
>>
>> Additionally the other architectures set kvm->online_vcpus to zero
>> after freeing them. We might as well do that for ARM too.
>
> Sure, I don't mind you removing the '-v' (verbose) from it. Should I
> respin? Or is that something you don't mind doing while applying?
>
No need to respin, I already applied your patch with the adjusted
commit message.
Thanks,
-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
