From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tetsuya Mukawa Subject: Re: [PATCH] vhost: Fix retrieval of numa information in PMD Date: Wed, 6 Apr 2016 16:28:25 +0900 Message-ID: <5704BA99.4050308@igel.co.jp> References: <1459872587-11655-1-git-send-email-ciara.loftus@intel.com> <5704B175.3040700@igel.co.jp> <20160406071754.GY3080@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: Ciara Loftus , dev@dpdk.org, "Tan, Jianfeng" To: Yuanhan Liu Return-path: Received: from mail-pf0-f180.google.com (mail-pf0-f180.google.com [209.85.192.180]) by dpdk.org (Postfix) with ESMTP id DBFB12C64 for ; Wed, 6 Apr 2016 09:28:28 +0200 (CEST) Received: by mail-pf0-f180.google.com with SMTP id e128so28004746pfe.3 for ; Wed, 06 Apr 2016 00:28:28 -0700 (PDT) In-Reply-To: <20160406071754.GY3080@yliu-dev.sh.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >>> --- >>> 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