Re: [dpdk-dev] [PATCH] vhost: Fix retrieval of numa information in PMD

[Tetsuya Mukawa <mukawa@igel.co.jp>]

On 2016/04/06 16:17, Yuanhan Liu wrote:
> On Wed, Apr 06, 2016 at 03:49:25PM +0900, Tetsuya Mukawa wrote:
>> On 2016/04/06 1:09, Ciara Loftus wrote:
>>> After some testing, it was found that retrieving numa information
>>> about a vhost device via a call to get_mempolicy is more
>>> accurate when performed during the new_device callback versus
>>> the vring_state_changed callback, in particular upon initial boot
>>> of the VM.  Performing this check during new_device is also
>>> potentially more efficient as this callback is only triggered once
>>> during device initialisation, compared with vring_state_changed
>>> which may be called multiple times depending on the number of
>>> queues assigned to the device.
>>>
>>> Reorganise the code to perform this check and assign the correct
>>> socket_id to the device during the new_device callback.
>>>
>>> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
>>> ---
>>>  drivers/net/vhost/rte_eth_vhost.c | 28 ++++++++++++++--------------
>>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
>>> index 4cc6bec..b1eb082 100644
>>> --- a/drivers/net/vhost/rte_eth_vhost.c
>>> +++ b/drivers/net/vhost/rte_eth_vhost.c
>>>
>> Hi,
>>
>> I appreciate fixing it.
>> Just one worry is that state changed event may be occurred before new
>> device event.
>> The users should not call rte_eth_dev_socket_id() until new device event
>> comes, even if they catch queue state events.
>> Otherwise, they will get wrong socket id to call
>> rte_eth_rx/tx_queue_setup().
> There is no way to guarantee that the socket id stuff would work
> perfectly in vhost, right? I mean, it's likely that virtio device
> would allocate memory from 2 or more sockets.
>
> So, it doesn't matter too much whether it's set perfectly right
> or not. Instead, we should assign it with a saner value instead
> of a obvious wrong one when new_device() is not invoked yet. So,
> I'd suggest to make an assignment first based on vhost_dev (or
> whatever) struct, and then make it "right" at new_device()
> callback?

Yes, I agree with you idea.

Thanks,
Tetsuya

>> So how about commenting it in 'rte_eth_vhost.h'?
> It asks a different usage than other PMDs, which I don't think
> it's a good idea.
>
> 	--yliu