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 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" Reviewed-by: Vitaly Kuznetsov Signed-off-by: Dexuan Cui --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
On Tue, Feb 3, 2015 at 11:40 AM, KY Srinivasan wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 7:38 PM To: KY Srinivasan Cc: Dexuan Cui; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; vkuzn...@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 wrote: > > >> -Original Message- >> From: Jason Wang [mailto:jasow...@redhat.com] >> Sent: Monday, February 2, 2015 7:09 PM >> To: Dexuan Cui >> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; >> driverdev- >> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY >> Srinivasan; vkuzn...@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 >> wrote: >> >> -Original Message- >> >> From: Jason Wang [mailto:jasow...@redhat.com] >> Sent: Monday, >> February 2, 2015 17:36 PM >> To: Dexuan Cui >> Cc: >> gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; >> >> driverdev- >> de...@linuxdriverproject.org; o...@aepfle.de; >> a...@canonical.com; KY >> Srinivasan; vkuzn...@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 >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
> -Original Message- > From: Jason Wang [mailto:jasow...@redhat.com] > Sent: Monday, February 2, 2015 7:09 PM > To: Dexuan Cui > Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY > Srinivasan; vkuzn...@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 wrote: > >> -Original Message- > >> From: Jason Wang [mailto:jasow...@redhat.com] > >> Sent: Monday, February 2, 2015 17:36 PM > >> To: Dexuan Cui > >> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > >> driverdev- > >> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY > >> Srinivasan; vkuzn...@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 > >> 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 > >
RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
> -Original Message- > From: Jason Wang [mailto:jasow...@redhat.com] > Sent: Monday, February 2, 2015 7:38 PM > To: KY Srinivasan > Cc: Dexuan Cui; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > driverdev-de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuzn...@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 > wrote: > > > > > >> -Original Message- > >> From: Jason Wang [mailto:jasow...@redhat.com] > >> Sent: Monday, February 2, 2015 7:09 PM > >> To: Dexuan Cui > >> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; > >> driverdev- > >> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY > >> Srinivasan; vkuzn...@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 > >> wrote: > >> >> -Original Message- > >> >> From: Jason Wang [mailto:jasow...@redhat.com] >> Sent: Monday, > >> February 2, 2015 17:36 PM >> To: Dexuan Cui >> Cc: > >> gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; >> > >> driverdev- >> de...@linuxdriverproject.org; o...@aepfle.de; > >> a...@canonical.com; KY >> Srinivasan; vkuzn...@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 >> 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 > >> > >> > >
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 wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 7:09 PM To: Dexuan Cui Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@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 wrote: >> -Original Message- >> From: Jason Wang [mailto:jasow...@redhat.com] >> Sent: Monday, February 2, 2015 17:36 PM >> To: Dexuan Cui >> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; >> driverdev- >> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY >> Srinivasan; vkuzn...@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 >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 17:36 PM To: Dexuan Cui Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Monday, February 2, 2015 17:36 PM
> To: Dexuan Cui
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY
> Srinivasan; vkuzn...@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 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"
> > Reviewed-by: Vitaly Kuznetsov
> > Signed-off-by: Dexuan Cui
> > ---
> >
> > 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�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
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 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" Reviewed-by: Vitaly Kuznetsov Signed-off-by: Dexuan Cui --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 de...@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 k...@microsoft.com Reviewed-by: Vitaly Kuznetsov vkuzn...@redhat.com Signed-off-by: Dexuan Cui de...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
-Original Message-
From: Jason Wang [mailto:jasow...@redhat.com]
Sent: Monday, February 2, 2015 17:36 PM
To: Dexuan Cui
Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY
Srinivasan; vkuzn...@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 de...@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 k...@microsoft.com
Reviewed-by: Vitaly Kuznetsov vkuzn...@redhat.com
Signed-off-by: Dexuan Cui de...@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�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
-Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 7:38 PM To: KY Srinivasan Cc: Dexuan Cui; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; vkuzn...@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 k...@microsoft.com wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 7:09 PM To: Dexuan Cui Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@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 de...@microsoft.com wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com]Sent: Monday, February 2, 2015 17:36 PMTo: Dexuan CuiCc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@redhat.com; Haiyang ZhangSubject: 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 de...@microsoft.com wrote: Before the line vmbus_open() returns, srv-util_cb can be alreadyrunning 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
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 k...@microsoft.com wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 7:09 PM To: Dexuan Cui Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@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 de...@microsoft.com wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 17:36 PM To: Dexuan Cui Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@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 de...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
-Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 7:09 PM To: Dexuan Cui Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@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 de...@microsoft.com wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 17:36 PM To: Dexuan Cui Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@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 de...@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
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 de...@microsoft.com wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 17:36 PM To: Dexuan Cui Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@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 de...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 de...@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 k...@microsoft.com Reviewed-by: Vitaly Kuznetsov vkuzn...@redhat.com Signed-off-by: Dexuan Cui de...@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 jasow...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
On Tue, Feb 3, 2015 at 11:40 AM, KY Srinivasan k...@microsoft.com wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 7:38 PM To: KY Srinivasan Cc: Dexuan Cui; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; vkuzn...@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 k...@microsoft.com wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Monday, February 2, 2015 7:09 PM To: Dexuan Cui Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev- de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@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 de...@microsoft.com wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com]Sent: Monday, February 2, 2015 17:36 PMTo: Dexuan CuiCc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY Srinivasan; vkuzn...@redhat.com; Haiyang ZhangSubject: 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 de...@microsoft.com wrote: Before the line vmbus_open() returns, srv-util_cb can be alreadyrunning 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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
