We'll do.
On Tue, 13 Aug 2019 10:44:28 +0200, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> On 09/08/19 17:59, Adalbert Lazăr wrote:
> > +static int kvmi_recv(void *arg)
> > +{
> > + struct kvmi *ikvm = arg;
> > +
> > + kvmi_info(ikvm, "Hooking VM\n");
> > +
> > + while (kvmi_msg_process(ikvm))
> > + ;
> > +
> > + kvmi_info(ikvm, "Unhooking VM\n");
> > +
> > + kvmi_end_introspection(ikvm);
> > +
> > + return 0;
> > +}
> > +
>
> Rename this to kvmi_recv_thread instead, please.
>
> > +
> > + /*
> > + * Make sure all the KVM/KVMI structures are linked and no pointer
> > + * is read as NULL after the reference count has been set.
> > + */
> > + smp_mb__before_atomic();
>
> This is an smp_wmb(), not an smp_mb__before_atomic(). Add a comment
> that it pairs with the refcount_inc_not_zero in kvmi_get.
>
> > + refcount_set(&kvm->kvmi_ref, 1);
> > +
>
>
> > @@ -57,8 +183,27 @@ void kvmi_destroy_vm(struct kvm *kvm)
> > if (!ikvm)
> > return;
> >
> > + /* trigger socket shutdown - kvmi_recv() will start shutdown process */
> > + kvmi_sock_shutdown(ikvm);
> > +
> > kvmi_put(kvm);
> >
> > /* wait for introspection resources to be released */
> > wait_for_completion_killable(&kvm->kvmi_completed);
> > }
> > +
>
> This addition means that kvmi_destroy_vm should have called
> kvmi_end_introspection instead. In patch 1, kvmi_end_introspection
> should have been just kvmi_put, now this patch can add kvmi_sock_shutdown.
>
> Paolo
