> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx] > > void hv_fcopy_deinit(void) > > { > > + bool wait = hvt->dev_opened; > > + > > fcopy_transaction.state = HVUTIL_DEVICE_DYING; > > cancel_delayed_work_sync(&fcopy_timeout_work); > > hvutil_transport_destroy(hvt); > > - wait_for_completion(&release_event); > > + if (wait) > > + wait_for_completion(&release_event); > > This is racy I think. We need to prevent openning the device first and > then query its state: > > bool wait; > > fcopy_transaction.state = HVUTIL_DEVICE_DYING; > /* make sure state is set */ > mb(); > wait = hvt->dev_opened; > cancel_delayed_work_sync(&fcopy_timeout_work); > hvutil_transport_destroy(hvt); > if (wait) > wait_for_completion(&release_event); > > otherwise someone could open the device before we manage to update its > state. I agree. > > @@ -182,6 +183,7 @@ static int hvt_op_release(struct inode *inode, > struct file *file) > > * connects back. > > */ > > hvt_reset(hvt); > > + hvt->dev_opened = false; > > mutex_unlock(&hvt->lock); > > > > Not sure but it seems this may also be racy (what if we query the state > just before we reset it?). Yeah, I agree. > > if (mode_old == HVUTIL_TRANSPORT_DESTROY) > > diff --git a/drivers/hv/hv_utils_transport.h > b/drivers/hv/hv_utils_transport.h > > index d98f522..9871283 100644 > > --- 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. :-) Thanks, -- Dexuan _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
