* [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place @ 2015-02-02 4:35 Dexuan Cui 2015-02-02 9:36 ` Jason Wang 2015-02-03 4:35 ` Jason Wang 0 siblings, 2 replies; 9+ messages in thread From: Dexuan Cui @ 2015-02-02 4:35 UTC (permalink / raw) To: gregkh, linux-kernel, driverdev-devel, olaf, apw, jasowang, kys, vkuznets Cc: haiyangz Before the line vmbus_open() returns, srv->util_cb can be already running and the variables, like util_fw_version, are needed by the srv->util_cb. So we have to make sure the variables are initialized before the vmbus_open(). CC: "K. Y. Srinivasan" <kys@microsoft.com> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: Dexuan Cui <decui@microsoft.com> --- v2: This is a RESEND. I just added Vitaly's Reviewed-by. drivers/hv/hv_util.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c index 3b9c9ef..c5be773 100644 --- a/drivers/hv/hv_util.c +++ b/drivers/hv/hv_util.c @@ -340,12 +340,8 @@ static int util_probe(struct hv_device *dev, set_channel_read_state(dev->channel, false); - ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 0, - srv->util_cb, dev->channel); - if (ret) - goto error; - hv_set_drvdata(dev, srv); + /* * Based on the host; initialize the framework and * service version numbers we will negotiate. @@ -365,6 +361,11 @@ static int util_probe(struct hv_device *dev, hb_srv_version = HB_VERSION; } + ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 0, + srv->util_cb, dev->channel); + if (ret) + goto error; + return 0; error: -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place 2015-02-02 4:35 [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place Dexuan Cui @ 2015-02-02 9:36 ` Jason Wang 2015-02-02 10:09 ` Dexuan Cui 2015-02-03 4:35 ` Jason Wang 1 sibling, 1 reply; 9+ messages in thread From: Jason Wang @ 2015-02-02 9:36 UTC (permalink / raw) To: Dexuan Cui Cc: gregkh, linux-kernel, driverdev-devel, olaf, apw, kys, vkuznets, haiyangz On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com> wrote: > Before the line vmbus_open() returns, srv->util_cb can be already > running > and the variables, like util_fw_version, are needed by the > srv->util_cb. A questions is why we do this for util only? Can callbacks of other devices be called before vmbus_open() returns? > > So we have to make sure the variables are initialized before the > vmbus_open(). > > CC: "K. Y. Srinivasan" <kys@microsoft.com> > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Dexuan Cui <decui@microsoft.com> > --- > > v2: > This is a RESEND. > I just added Vitaly's Reviewed-by. > > drivers/hv/hv_util.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c > index 3b9c9ef..c5be773 100644 > --- a/drivers/hv/hv_util.c > +++ b/drivers/hv/hv_util.c > @@ -340,12 +340,8 @@ static int util_probe(struct hv_device *dev, > > set_channel_read_state(dev->channel, false); > > - ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, > 0, > - srv->util_cb, dev->channel); > - if (ret) > - goto error; > - > hv_set_drvdata(dev, srv); > + This seems unnecessary. > > /* > * Based on the host; initialize the framework and > * service version numbers we will negotiate. > @@ -365,6 +361,11 @@ static int util_probe(struct hv_device *dev, > hb_srv_version = HB_VERSION; > } > > + ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, > 0, > + srv->util_cb, dev->channel); > + if (ret) > + goto error; > + > return 0; > > error: > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place 2015-02-02 9:36 ` Jason Wang @ 2015-02-02 10:09 ` Dexuan Cui [not found] ` <F792CF86EFE20D4AB8064279AFBA51C61F32EF4E@HKNPRD3002MB017.064d.mgd.msft.net > 0 siblings, 1 reply; 9+ messages in thread From: Dexuan Cui @ 2015-02-02 10:09 UTC (permalink / raw) To: Jason Wang Cc: gregkh, linux-kernel, driverdev-devel, olaf, apw, KY Srinivasan, vkuznets, Haiyang Zhang [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2653 bytes --] > -----Original Message----- > From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Monday, February 2, 2015 17:36 PM > To: Dexuan Cui > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY > Srinivasan; vkuznets@redhat.com; Haiyang Zhang > Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place > > > > On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com> wrote: > > Before the line vmbus_open() returns, srv->util_cb can be already > > running > > and the variables, like util_fw_version, are needed by the > > srv->util_cb. > > A questions is why we do this for util only? Can callbacks of other > devices be called before vmbus_open() returns? The variables are used in vmbus_prep_negotiate_resp(), which is only for the util devices. I think the other devices should already handle the similar issue properly. If this is not the case, we need to fix them too. > > > > > So we have to make sure the variables are initialized before the > > vmbus_open(). > > > > CC: "K. Y. Srinivasan" <kys@microsoft.com> > > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > > Signed-off-by: Dexuan Cui <decui@microsoft.com> > > --- > > > > v2: > > This is a RESEND. > > I just added Vitaly's Reviewed-by. > > > > drivers/hv/hv_util.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c > > index 3b9c9ef..c5be773 100644 > > --- a/drivers/hv/hv_util.c > > +++ b/drivers/hv/hv_util.c > > @@ -340,12 +340,8 @@ static int util_probe(struct hv_device *dev, > > > > set_channel_read_state(dev->channel, false); > > > > - ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, > > 0, > > - srv->util_cb, dev->channel); > > - if (ret) > > - goto error; > > - > > hv_set_drvdata(dev, srv); > > + > > This seems unnecessary. Yeah, it's an empty line, splitting the line and the below comment. I'm Ok to not have it. > > /* > > * Based on the host; initialize the framework and > > * service version numbers we will negotiate. > > @@ -365,6 +361,11 @@ static int util_probe(struct hv_device *dev, > > hb_srv_version = HB_VERSION; > > } > > > > + ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, > > 0, > > + srv->util_cb, dev->channel); > > + if (ret) > > + goto error; > > + > > return 0; > > > > error: > > -- > > 1.9.1 > > -- Dexuan ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <F792CF86EFE20D4AB8064279AFBA51C61F32EF4E@HKNPRD3002MB017.064d.mgd.msft.net >]
* RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place [not found] ` <F792CF86EFE20D4AB8064279AFBA51C61F32EF4E@HKNPRD3002MB017.064d.mgd.msft.net > @ 2015-02-03 3:08 ` Jason Wang 2015-02-03 3:30 ` KY Srinivasan 0 siblings, 1 reply; 9+ messages in thread From: Jason Wang @ 2015-02-03 3:08 UTC (permalink / raw) To: Dexuan Cui Cc: gregkh, linux-kernel, driverdev-devel, olaf, apw, KY Srinivasan, vkuznets, Haiyang Zhang On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com> wrote: >> -----Original Message----- >> From: Jason Wang [mailto:jasowang@redhat.com] >> Sent: Monday, February 2, 2015 17:36 PM >> To: Dexuan Cui >> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; >> driverdev- >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang >> Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a >> later place >> >> >> >> On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com> >> wrote: >> > Before the line vmbus_open() returns, srv->util_cb can be already >> > running >> > and the variables, like util_fw_version, are needed by the >> > srv->util_cb. >> >> A questions is why we do this for util only? Can callbacks of other >> devices be called before vmbus_open() returns? > The variables are used in vmbus_prep_negotiate_resp(), which is only > for > the util devices. > > I think the other devices should already handle the similar issue > properly. > If this is not the case, we need to fix them too. Better to check all the others, e.g in balloon_probe(), it call hv_set_drvdata() after vmbus_open() and dose several datas setups in the middle. If balloon_onchannelcallback() could be called before hv_set_drvdata(), the code looks wrong. Thanks ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place 2015-02-03 3:08 ` Jason Wang @ 2015-02-03 3:30 ` KY Srinivasan [not found] ` <BY2PR0301MB071153EA9F73C9855A4B4119A03D0@BY2PR0301MB0711.namprd03.prod.out look.com> 0 siblings, 1 reply; 9+ messages in thread From: KY Srinivasan @ 2015-02-03 3:30 UTC (permalink / raw) To: Jason Wang, Dexuan Cui Cc: gregkh, linux-kernel, driverdev-devel, olaf, apw, vkuznets, Haiyang Zhang [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2388 bytes --] > -----Original Message----- > From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Monday, February 2, 2015 7:09 PM > To: Dexuan Cui > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY > Srinivasan; vkuznets@redhat.com; Haiyang Zhang > Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place > > > > On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com> wrote: > >> -----Original Message----- > >> From: Jason Wang [mailto:jasowang@redhat.com] > >> Sent: Monday, February 2, 2015 17:36 PM > >> To: Dexuan Cui > >> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > >> driverdev- > >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY > >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang > >> Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a > >> later place > >> > >> > >> > >> On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com> > >> wrote: > >> > Before the line vmbus_open() returns, srv->util_cb can be already > >> > running > and the variables, like util_fw_version, are needed by > >> the > srv->util_cb. > >> > >> A questions is why we do this for util only? Can callbacks of other > >> devices be called before vmbus_open() returns? > > The variables are used in vmbus_prep_negotiate_resp(), which is only > > for the util devices. > > > > I think the other devices should already handle the similar issue > > properly. > > If this is not the case, we need to fix them too. > > Better to check all the others, e.g in balloon_probe(), it call > hv_set_drvdata() after vmbus_open() and dose several datas setups in the > middle. If balloon_onchannelcallback() could be called before > hv_set_drvdata(), the code looks wrong. Jason, For all other device types, the guest initiates the communication with the host and potentially negotiates appropriate (supported) version with the host. For the services packaged in the util driver, the flow is a little different - the host pushes the version information into the guest. So, the fix Dexuan made is only needed in the util driver. Regards, K. Y > > Thanks > > ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <BY2PR0301MB071153EA9F73C9855A4B4119A03D0@BY2PR0301MB0711.namprd03.prod.out look.com>]
* RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place [not found] ` <BY2PR0301MB071153EA9F73C9855A4B4119A03D0@BY2PR0301MB0711.namprd03.prod.out look.com> @ 2015-02-03 3:38 ` Jason Wang 2015-02-03 3:40 ` KY Srinivasan 0 siblings, 1 reply; 9+ messages in thread From: Jason Wang @ 2015-02-03 3:38 UTC (permalink / raw) To: KY Srinivasan Cc: Dexuan Cui, gregkh, linux-kernel, driverdev-devel, olaf, apw, vkuznets, Haiyang Zhang On Tue, Feb 3, 2015 at 11:30 AM, KY Srinivasan <kys@microsoft.com> wrote: > > >> -----Original Message----- >> From: Jason Wang [mailto:jasowang@redhat.com] >> Sent: Monday, February 2, 2015 7:09 PM >> To: Dexuan Cui >> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; >> driverdev- >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang >> Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a >> later place >> >> >> >> On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com> >> wrote: >> >> -----Original Message----- >> >> From: Jason Wang [mailto:jasowang@redhat.com] >> >> Sent: Monday, February 2, 2015 17:36 PM >> >> To: Dexuan Cui >> >> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; >> >> driverdev- >> >> devel@linuxdriverproject.org; olaf@aepfle.de; >> apw@canonical.com; KY >> >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang >> >> Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a >> >> later place >> >> >> >> >> >> >> >> On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui >> <decui@microsoft.com> >> >> wrote: >> >> > Before the line vmbus_open() returns, srv->util_cb can be >> already >> >> > running > and the variables, like util_fw_version, are needed >> by >> >> the > srv->util_cb. >> >> >> >> A questions is why we do this for util only? Can callbacks of >> other >> >> devices be called before vmbus_open() returns? >> > The variables are used in vmbus_prep_negotiate_resp(), which is >> only >> > for the util devices. >> > >> > I think the other devices should already handle the similar issue >> > properly. >> > If this is not the case, we need to fix them too. >> >> Better to check all the others, e.g in balloon_probe(), it call >> hv_set_drvdata() after vmbus_open() and dose several datas setups >> in the >> middle. If balloon_onchannelcallback() could be called before >> hv_set_drvdata(), the code looks wrong. > > Jason, > > For all other device types, the guest initiates the communication > with the host and potentially > negotiates appropriate (supported) version with the host. For the > services packaged in the util > driver, the flow is a little different - the host pushes the version > information into the guest. So, > the fix Dexuan made is only needed in the util driver. > > Regards, > > K. Y Thanks, so you mean for other device, it won't get any interrupt before guest negotiate the version with host? > >> >> Thanks >> >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place 2015-02-03 3:38 ` Jason Wang @ 2015-02-03 3:40 ` KY Srinivasan [not found] ` <BY2PR0301MB07113EC0C4277FA01AE66B9DA03D0@BY2PR0301MB0711.namprd03.prod.out look.com> 0 siblings, 1 reply; 9+ messages in thread From: KY Srinivasan @ 2015-02-03 3:40 UTC (permalink / raw) To: Jason Wang Cc: Dexuan Cui, gregkh, linux-kernel, driverdev-devel, olaf, apw, vkuznets, Haiyang Zhang [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3311 bytes --] > -----Original Message----- > From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Monday, February 2, 2015 7:38 PM > To: KY Srinivasan > Cc: Dexuan Cui; gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > driverdev-devel@linuxdriverproject.org; olaf@aepfle.de; > apw@canonical.com; vkuznets@redhat.com; Haiyang Zhang > Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place > > > > On Tue, Feb 3, 2015 at 11:30 AM, KY Srinivasan <kys@microsoft.com> > wrote: > > > > > >> -----Original Message----- > >> From: Jason Wang [mailto:jasowang@redhat.com] > >> Sent: Monday, February 2, 2015 7:09 PM > >> To: Dexuan Cui > >> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > >> driverdev- > >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY > >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang > >> Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a > >> later place > >> > >> > >> > >> On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com> > >> wrote: > >> >> -----Original Message----- > >> >> From: Jason Wang [mailto:jasowang@redhat.com] >> Sent: Monday, > >> February 2, 2015 17:36 PM >> To: Dexuan Cui >> Cc: > >> gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; >> > >> driverdev- >> devel@linuxdriverproject.org; olaf@aepfle.de; > >> apw@canonical.com; KY >> Srinivasan; vkuznets@redhat.com; Haiyang > >> Zhang >> Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() > >> to a >> later place >> >> >> >> On Mon, Feb 2, 2015 at 12:35 > >> PM, Dexuan Cui <decui@microsoft.com> >> wrote: > >> >> > Before the line vmbus_open() returns, srv->util_cb can be > >> already >> > running > and the variables, like util_fw_version, are > >> needed by >> the > srv->util_cb. > >> >> > >> >> A questions is why we do this for util only? Can callbacks of > >> other >> devices be called before vmbus_open() returns? > >> > The variables are used in vmbus_prep_negotiate_resp(), which is > >> only > for the util devices. > >> > > >> > I think the other devices should already handle the similar issue > >> > properly. > >> > If this is not the case, we need to fix them too. > >> > >> Better to check all the others, e.g in balloon_probe(), it call > >> hv_set_drvdata() after vmbus_open() and dose several datas setups in > >> the middle. If balloon_onchannelcallback() could be called before > >> hv_set_drvdata(), the code looks wrong. > > > > Jason, > > > > For all other device types, the guest initiates the communication with > > the host and potentially negotiates appropriate (supported) version > > with the host. For the services packaged in the util driver, the flow > > is a little different - the host pushes the version information into > > the guest. So, the fix Dexuan made is only needed in the util driver. > > > > Regards, > > > > K. Y > > Thanks, so you mean for other device, it won't get any interrupt before guest > negotiate the version with host? Yes, the guest initiates the version negotiation. Are you seeing something different? K. Y > > > >> > >> Thanks > >> > >> > > ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <BY2PR0301MB07113EC0C4277FA01AE66B9DA03D0@BY2PR0301MB0711.namprd03.prod.out look.com>]
* RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place [not found] ` <BY2PR0301MB07113EC0C4277FA01AE66B9DA03D0@BY2PR0301MB0711.namprd03.prod.out look.com> @ 2015-02-03 4:35 ` Jason Wang 0 siblings, 0 replies; 9+ messages in thread From: Jason Wang @ 2015-02-03 4:35 UTC (permalink / raw) To: KY Srinivasan Cc: Dexuan Cui, gregkh, linux-kernel, driverdev-devel, olaf, apw, vkuznets, Haiyang Zhang On Tue, Feb 3, 2015 at 11:40 AM, KY Srinivasan <kys@microsoft.com> wrote: > > >> -----Original Message----- >> From: Jason Wang [mailto:jasowang@redhat.com] >> Sent: Monday, February 2, 2015 7:38 PM >> To: KY Srinivasan >> Cc: Dexuan Cui; gregkh@linuxfoundation.org; >> linux-kernel@vger.kernel.org; >> driverdev-devel@linuxdriverproject.org; olaf@aepfle.de; >> apw@canonical.com; vkuznets@redhat.com; Haiyang Zhang >> Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a >> later place >> >> >> >> On Tue, Feb 3, 2015 at 11:30 AM, KY Srinivasan <kys@microsoft.com> >> wrote: >> > >> > >> >> -----Original Message----- >> >> From: Jason Wang [mailto:jasowang@redhat.com] >> >> Sent: Monday, February 2, 2015 7:09 PM >> >> To: Dexuan Cui >> >> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; >> >> driverdev- >> >> devel@linuxdriverproject.org; olaf@aepfle.de; >> apw@canonical.com; KY >> >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang >> >> Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a >> >> later place >> >> >> >> >> >> >> >> On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com> >> >> wrote: >> >> >> -----Original Message----- >> >> >> From: Jason Wang [mailto:jasowang@redhat.com] >> Sent: >> Monday, >> >> February 2, 2015 17:36 PM >> To: Dexuan Cui >> Cc: >> >> gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; >> >> >> driverdev- >> devel@linuxdriverproject.org; olaf@aepfle.de; >> >> apw@canonical.com; KY >> Srinivasan; vkuznets@redhat.com; >> Haiyang >> >> Zhang >> Subject: Re: [PATCH v2 1/3] hv: hv_util: move >> vmbus_open() >> >> to a >> later place >> >> >> >> On Mon, Feb 2, 2015 at >> 12:35 >> >> PM, Dexuan Cui <decui@microsoft.com> >> wrote: >> >> >> > Before the line vmbus_open() returns, srv->util_cb can be >> >> already >> > running > and the variables, like >> util_fw_version, are >> >> needed by >> the > srv->util_cb. >> >> >> >> >> >> A questions is why we do this for util only? Can callbacks >> of >> >> other >> devices be called before vmbus_open() returns? >> >> > The variables are used in vmbus_prep_negotiate_resp(), which >> is >> >> only > for the util devices. >> >> > >> >> > I think the other devices should already handle the similar >> issue >> >> > properly. >> >> > If this is not the case, we need to fix them too. >> >> >> >> Better to check all the others, e.g in balloon_probe(), it call >> >> hv_set_drvdata() after vmbus_open() and dose several datas >> setups in >> >> the middle. If balloon_onchannelcallback() could be called >> before >> >> hv_set_drvdata(), the code looks wrong. >> > >> > Jason, >> > >> > For all other device types, the guest initiates the communication >> with >> > the host and potentially negotiates appropriate (supported) >> version >> > with the host. For the services packaged in the util driver, the >> flow >> > is a little different - the host pushes the version information >> into >> > the guest. So, the fix Dexuan made is only needed in the util >> driver. >> > >> > Regards, >> > >> > K. Y >> >> Thanks, so you mean for other device, it won't get any interrupt >> before guest >> negotiate the version with host? > > Yes, the guest initiates the version negotiation. Are you seeing > something different? > > K. Y No, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place 2015-02-02 4:35 [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place Dexuan Cui 2015-02-02 9:36 ` Jason Wang @ 2015-02-03 4:35 ` Jason Wang 1 sibling, 0 replies; 9+ messages in thread From: Jason Wang @ 2015-02-03 4:35 UTC (permalink / raw) To: Dexuan Cui Cc: gregkh, linux-kernel, driverdev-devel, olaf, apw, kys, vkuznets, haiyangz On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com> wrote: > Before the line vmbus_open() returns, srv->util_cb can be already > running > and the variables, like util_fw_version, are needed by the > srv->util_cb. > > So we have to make sure the variables are initialized before the > vmbus_open(). > > CC: "K. Y. Srinivasan" <kys@microsoft.com> > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Dexuan Cui <decui@microsoft.com> > --- > > v2: > This is a RESEND. > I just added Vitaly's Reviewed-by. > > drivers/hv/hv_util.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c > index 3b9c9ef..c5be773 100644 > --- a/drivers/hv/hv_util.c > +++ b/drivers/hv/hv_util.c > @@ -340,12 +340,8 @@ static int util_probe(struct hv_device *dev, > > set_channel_read_state(dev->channel, false); > > - ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, > 0, > - srv->util_cb, dev->channel); > - if (ret) > - goto error; > - > hv_set_drvdata(dev, srv); > + > /* > * Based on the host; initialize the framework and > * service version numbers we will negotiate. > @@ -365,6 +361,11 @@ static int util_probe(struct hv_device *dev, > hb_srv_version = HB_VERSION; > } > > + ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, > 0, > + srv->util_cb, dev->channel); > + if (ret) > + goto error; > + > return 0; > > error: > -- > 1.9.1 Reviewed-by: Jason Wang <jasowang@redhat.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-02-03 4:36 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-02 4:35 [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place Dexuan Cui
2015-02-02 9:36 ` Jason Wang
2015-02-02 10:09 ` Dexuan Cui
[not found] ` <F792CF86EFE20D4AB8064279AFBA51C61F32EF4E@HKNPRD3002MB017.064d.mgd.msft.net >
2015-02-03 3:08 ` Jason Wang
2015-02-03 3:30 ` KY Srinivasan
[not found] ` <BY2PR0301MB071153EA9F73C9855A4B4119A03D0@BY2PR0301MB0711.namprd03.prod.out look.com>
2015-02-03 3:38 ` Jason Wang
2015-02-03 3:40 ` KY Srinivasan
[not found] ` <BY2PR0301MB07113EC0C4277FA01AE66B9DA03D0@BY2PR0301MB0711.namprd03.prod.out look.com>
2015-02-03 4:35 ` Jason Wang
2015-02-03 4:35 ` Jason Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).