> -----Original Message----- > From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx] > Sent: Thursday, February 18, 2016 5:35 AM > To: devel@xxxxxxxxxxxxxxxxxxxxxx > Cc: KY Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang Zhang > <haiyangz@xxxxxxxxxxxxx>; Olaf Hering <olaf@xxxxxxxxx>; Cathy Avery > <cavery@xxxxxxxxxx> > Subject: [PATCH] Drivers: hv: util: fix a race with daemons startup > > Commit 3cace4a61610 ("Drivers: hv: utils: run polling callback always in > interrupt context") removed direct *_transaction.state = HVUTIL_READY > assignments from *_handle_handshake() functions introducing the following > race: if a userspace daemon connects before we get first non-negotiation > request from the server hv_poll_channel() won't set transaction state to > HVUTIL_READY as (!channel) condition will fail, we set it to non-NULL on > the first real request from the server. Solve the issue by transferring > the transaction state to HVUTIL_READY directly from all handshake > functions. > > Fixes: 3cace4a61610 ("Drivers: hv: utils: run polling callback always in > interrupt context") > Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> > --- > drivers/hv/hv_fcopy.c | 1 + > drivers/hv/hv_kvp.c | 1 + > drivers/hv/hv_snapshot.c | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c > index c37a71e..e18b85b 100644 > --- a/drivers/hv/hv_fcopy.c > +++ b/drivers/hv/hv_fcopy.c > @@ -108,6 +108,7 @@ static int fcopy_handle_handshake(u32 version) > return -EINVAL; > } > pr_debug("FCP: userspace daemon ver. %d registered\n", version); > + fcopy_transaction.state = HVUTIL_READY; > hv_poll_channel(fcopy_transaction.recv_channel, > fcopy_poll_wrapper); > return 0; > } > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c > index d4ab81b..1162afb 100644 > --- a/drivers/hv/hv_kvp.c > +++ b/drivers/hv/hv_kvp.c > @@ -154,6 +154,7 @@ static int kvp_handle_handshake(struct hv_kvp_msg > *msg) > pr_debug("KVP: userspace daemon ver. %d registered\n", > KVP_OP_REGISTER); > kvp_register(dm_reg_value); > + kvp_transaction.state = HVUTIL_READY; > hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper); > > return 0; > diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c > index 67def4a..489203b 100644 > --- a/drivers/hv/hv_snapshot.c > +++ b/drivers/hv/hv_snapshot.c > @@ -113,6 +113,7 @@ static int vss_handle_handshake(struct hv_vss_msg > *vss_msg) > default: > return -EINVAL; > } > + vss_transaction.state = HVUTIL_READY; > hv_poll_channel(vss_transaction.recv_channel, vss_poll_wrapper); > pr_debug("VSS: userspace daemon ver. %d registered\n", > dm_reg_value); > return 0; Vitaly, there is a simpler and a safer fix to this issue - stash away the channel pointer early on and so we don't need to set it on each transaction. This would address the issue on hand here. This would also eliminate setting the state only in the interrupt handler. I am putting together a batch of patches to send and if it is ok with you, I will drop this patch and include a patch on the lines described above. Regards, K. Y > -- > 2.5.0 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
