> -----Original Message----- > From: Dexuan Cui > Sent: Tuesday, January 13, 2015 5:44 PM > To: Vitaly Kuznetsov; KY Srinivasan > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; driverdev- > devel@xxxxxxxxxxxxxxxxxxxxxx; olaf@xxxxxxxxx; apw@xxxxxxxxxxxxx; > jasowang@xxxxxxxxxx; Haiyang Zhang > Subject: RE: [PATCH v3] hv: hv_fcopy: drop the obsolete message on transfer > failure > > > -----Original Message----- > > From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx] > > Sent: Tuesday, January 13, 2015 21:52 PM > > To: Dexuan Cui; KY Srinivasan > > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > > driverdev- devel@xxxxxxxxxxxxxxxxxxxxxx; olaf@xxxxxxxxx; > > apw@xxxxxxxxxxxxx; jasowang@xxxxxxxxxx; Haiyang Zhang > > Subject: Re: [PATCH v3] hv: hv_fcopy: drop the obsolete message on > > transfer failure > > > > Dexuan Cui <decui@xxxxxxxxxxxxx> writes: > > > > > In the case the user-space daemon crashes, hangs or is killed, we > > > need to down the semaphore, otherwise, after the daemon starts next > > > time, the obsolete data in fcopy_transaction.message or > > > fcopy_transaction.fcopy_msg will be used immediately. > > > > It seems this patch got lost and I don't see it in recent 'resend' > > series. K.Y., Dexuan, can you please take a look? > Hi Vitaly, Jason, > The patch can't fix all the corner cases (it would need non-trivial efforts for > that) as we discussed, but I think it would be better for us to have it as it can > indeed fix an obvious issue and doesn't introduce new issues. > > I think I can document the known discussed corner cases in the patch as > TODOs and resend the patch. > > Please let me know if you have different opinions. :-) Resend the patch. K. Y > > Thanks, > -- Dexuan > > > > > > > > > Cc: Jason Wang <jasowang@xxxxxxxxxx> > > > Cc: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> > > > Cc: K. Y. Srinivasan <kys@xxxxxxxxxxxxx> > > > Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx> > > > --- > > > > > > v2: I removed the "FCP" prefix as Greg asked. > > > > > > I also updated the output message a little: > > > "FCP: failed to acquire the semaphore" --> > > > "can not acquire the semaphore: it is benign" > > > > > > v3: I added the code in fcopy_release() as Jason Wang suggested. > > > I removed the pr_debug (it isn't so meaningful)and added a > > > comment > > instead. > > > > > > drivers/hv/hv_fcopy.c | 19 +++++++++++++++++++ > > > 1 file changed, 19 insertions(+) > > > > > > diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c index > > > 23b2ce2..faa6ba6 100644 > > > --- a/drivers/hv/hv_fcopy.c > > > +++ b/drivers/hv/hv_fcopy.c > > > @@ -86,6 +86,18 @@ static void fcopy_work_func(struct work_struct > > *dummy) > > > * process the pending transaction. > > > */ > > > fcopy_respond_to_host(HV_E_FAIL); > > > + > > > + /* In the case the user-space daemon crashes, hangs or is killed, we > > > + * need to down the semaphore, otherwise, after the daemon starts > > next > > > + * time, the obsolete data in fcopy_transaction.message or > > > + * fcopy_transaction.fcopy_msg will be used immediately. > > > + * > > > + * NOTE: fcopy_read() happens to get the semaphore (very rare)? > We're > > > + * still OK, because we've reported the failure to the host. > > > + */ > > > + if (down_trylock(&fcopy_transaction.read_sema)) > > > + ; > > > + > > > } > > > > > > static int fcopy_handle_handshake(u32 version) @@ -351,6 +363,13 @@ > > > static int fcopy_release(struct inode *inode, struct > > file *f) > > > */ > > > in_hand_shake = true; > > > opened = false; > > > + > > > + if (cancel_delayed_work_sync(&fcopy_work)) { > > > + /* We haven't up()-ed the semaphore(very rare)? */ > > > + if (down_trylock(&fcopy_transaction.read_sema)) > > > + ; > > > + fcopy_respond_to_host(HV_E_FAIL); > > > + } > > > return 0; > > > } > > > > -- > > Vitaly _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel