From: Xiao Liang <xiliang@redhat.com>
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>,
netdev@vger.kernel.org, xen-devel@lists.xenproject.org,
davem@davemloft.net, jgross@suse.com
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] xen-netfront: wait xenbus state change when load module manually
Date: Mon, 30 Jul 2018 15:43:45 +0800 [thread overview]
Message-ID: <41022efa-141a-01d9-3084-8460b5017592__18290.3431276374$1532936559$gmane$org@redhat.com> (raw)
In-Reply-To: <4dfca465-8da3-7ebf-aac1-08ffe34a74ac@oracle.com>
Thanks, Boris
Please see my reply inline.
On 07/28/2018 02:40 AM, Boris Ostrovsky wrote:
> On 07/27/2018 05:56 AM, Xiao Liang wrote:
>> When loading module manually, after call xenbus_switch_state to initializes
>> the state of the netfront device, the driver state did not change so fast
>> that may lead no dev created in latest kernel. This patch adds wait to make
>> sure xenbus knows the driver is not in closed/unknown state.
>>
>> Current state:
>> [vm]# ethtool eth0
>> Settings for eth0:
>> Link detected: yes
>> [vm]# modprobe -r xen_netfront
>> [vm]# modprobe xen_netfront
>> [vm]# ethtool eth0
>> Settings for eth0:
>> Cannot get device settings: No such device
>> Cannot get wake-on-lan settings: No such device
>> Cannot get message level: No such device
>> Cannot get link status: No such device
>> No data available
>>
>> With the patch installed.
>> [vm]# ethtool eth0
>> Settings for eth0:
>> Link detected: yes
>> [vm]# modprobe -r xen_netfront
>> [vm]# modprobe xen_netfront
>> [vm]# ethtool eth0
>> Settings for eth0:
>> Link detected: yes
>>
>> Signed-off-by: Xiao Liang <xiliang@redhat.com>
>> ---
>> drivers/net/xen-netfront.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>> index a57daecf1d57..2d8812dd1534 100644
>> --- a/drivers/net/xen-netfront.c
>> +++ b/drivers/net/xen-netfront.c
>> @@ -87,6 +87,7 @@ struct netfront_cb {
>> /* IRQ name is queue name with "-tx" or "-rx" appended */
>> #define IRQ_NAME_SIZE (QUEUE_NAME_SIZE + 3)
>>
>> +static DECLARE_WAIT_QUEUE_HEAD(module_load_q);
>> static DECLARE_WAIT_QUEUE_HEAD(module_unload_q);
>>
>> struct netfront_stats {
>> @@ -1330,6 +1331,11 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)
>> netif_carrier_off(netdev);
>>
>> xenbus_switch_state(dev, XenbusStateInitialising);
>> + wait_event(module_load_q,
>> + xenbus_read_driver_state(dev->otherend) !=
>> + XenbusStateClosed &&
>> + xenbus_read_driver_state(dev->otherend) !=
>> + XenbusStateUnknown);
>> return netdev;
>>
>> exit:
>
> Should we have a wake_up somewhere?
In my understanding, netback_changed handles it if dev state is in
XenbusStateInitialising and otherend is in XenbusStateInitWait, and then
create connection to backend.
But in most cases, it breaks out as dev->state not in
XenbusStateInitialising. So I added a wait here.
> And what about other states --- is,
> for example, XenbusStateClosing a valid reason to continue?
I think XenbusStateClosing should not be a valid reason to continue.
My purpose is waiting otherend status to be XenbusStateInitWait(after
new dev created).To avoid unnecessary impact, I only check it leaves
closed and unknow state in this patch.
In my testing, hotplug vifs from guest in host or load/unload module in
guest over 100 times, only waiting XenbusStateInitWait or as this patch
does, both are working.
vifs can be created each time successfully.
Thanks,
Xiao Liang
>
>
> -boris
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2018-07-30 7:43 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-27 9:56 [PATCH] xen-netfront: wait xenbus state change when load module manually Xiao Liang
2018-07-27 18:40 ` Boris Ostrovsky
2018-07-30 7:43 ` Xiao Liang [this message]
2018-07-30 7:43 ` [Xen-devel] " Xiao Liang
2018-07-27 18:40 ` Boris Ostrovsky
2018-07-29 15:30 ` David Miller
2018-07-29 15:30 ` David Miller
2018-07-30 8:18 ` [Xen-devel] " Xiao Liang
2018-08-24 11:12 ` Jiri Slaby
2018-08-24 11:26 ` Juergen Gross
2018-08-24 14:26 ` Boris Ostrovsky
2018-08-24 14:26 ` [Xen-devel] " Boris Ostrovsky
2018-09-07 11:06 ` Jiri Slaby
2018-09-07 11:30 ` Juergen Gross
2018-09-07 11:30 ` Juergen Gross
2018-09-07 11:06 ` Jiri Slaby
2018-08-24 11:26 ` Juergen Gross
2018-08-24 11:12 ` Jiri Slaby
2018-07-30 8:18 ` Xiao Liang
2018-07-30 16:41 ` David Miller
2018-07-30 16:41 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2018-07-27 9:56 Xiao Liang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='41022efa-141a-01d9-3084-8460b5017592__18290.3431276374$1532936559$gmane$org@redhat.com' \
--to=xiliang@redhat.com \
--cc=boris.ostrovsky@oracle.com \
--cc=davem@davemloft.net \
--cc=jgross@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.