Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [Xen-devel] [PATCH] xen-netfront: wait xenbus state change when
 load module manually
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
References: <20180727095608.25210-1-xiliang@redhat.com>
 <4dfca465-8da3-7ebf-aac1-08ffe34a74ac@oracle.com>
From:   Xiao Liang <xiliang@redhat.com>
Message-ID: <41022efa-141a-01d9-3084-8460b5017592@redhat.com>
Date:   Mon, 30 Jul 2018 15:43:45 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <4dfca465-8da3-7ebf-aac1-08ffe34a74ac@oracle.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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