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

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Wed, Apr 06, 2016 at 06:12:07PM +0200, Thomas Monjalon wrote:
> 2016-04-07 00:09, Yuanhan Liu:
> > On Wed, Apr 06, 2016 at 09:37:53AM +0000, Loftus, Ciara wrote:
> > > > On Wed, Apr 06, 2016 at 03:49:25PM +0900, Tetsuya Mukawa wrote:
> > > > > 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?
> > > 
> > > Thanks for the feedback.
> > > At the moment with this patch numa_node is initially set to rte_socket_id() during  pmd init and then updated to the correct value during new_device.
> > > Are you suggesting we set it again in between these two steps ("based on vhost_dev")? If so where do you think would be a good place?
> > 
> > Oh, I was not aware of that. Then I think we are fine here.
> 
> Please Yuanhan, could you be more explicit?

Oh, sorry, I made a reply at wrong place. My reply was against to
following statement:

  > At the moment with this patch numa_node is initially set to
  > rte_socket_id() during  pmd init and then updated to the correct
  > value during new_device.

So, we don't need do another numa_node inital assignment. Put simply,
this patch is fine.

	--yliu