Re: [PATCH] Drivers: hv: util: on deinit, don't wait the release event, if we shouldn't

From: Vitaly Kuznetsov
Date: Tue Feb 28 2017 - 09:22:30 EST


Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> writes:

> Dexuan Cui <decui@xxxxxxxxxxxxx> writes:
>
>>> From: devel [...] On Behalf Of Dexuan Cui
>>> > > --- a/drivers/hv/hv_utils_transport.h
>>> > > +++ b/drivers/hv/hv_utils_transport.h
>>> > > @@ -32,6 +32,7 @@ struct hvutil_transport {
>>> > > int mode; /* hvutil_transport_mode */
>>> > > struct file_operations fops; /* file operations */
>>> > > struct miscdevice mdev; /* misc device */
>>> > > + bool dev_opened; /* Is the device opened? */
>>> > > struct cb_id cn_id; /* CN_*_IDX/CN_*_VAL */
>>> > > struct list_head list; /* hvt_list */
>>> > > int (*on_msg)(void *, int); /* callback on new user message */
>>> >
>>> > I think we can get away without introducing this new flag, e.g. if we
>>> > replace release_event with an atomic which will hold the state
>>> > (open/closed). This will also elimenate possible races above. I can try
>>> > prototyping a patch if you want me to.
>>> > --
>>> > Vitaly
>>>
>>> Thanks for offering the help! Please do. :-)
>>
>> BTW, IMO I found another potential issue:
>> In hvt_op_open -> hvt_reset -> kvp_on_reset(), I think we should call
>> init_completion() instead of complete()?
>>
>
> To me it looks like we can do better with something different from
> struct completion, I'll take a look later today.

Dexuan,

please take a look at the attached patch. After looking at the code
again it occured to me that it's going to be easier to move release wait
to the transport itself. Lightly tested.

Thanks!

--
Vitaly