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:
> > > 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 at 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?
>
> 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.
--yliu
