On Thu, Jan 22, 2015 at 09:56:48AM +0000, Ferriter, Cian wrote:
> Hey guys,
> 
> I just wanted to ask is there anything more that can be done with this patch 
> or is it in an acceptable state for pushing?
> 
> Cian

At this stage I think I'm ok with the patch contents, unless anyone else 
objects.
However, your patch submission is missing the sign-off line needed before it
can be committed. Can you please resubmit with the proper sign-off. [See 
http://www.dpdk.org/dev]

Regards,
/Bruce

> 
> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ferriter, Cian
> Sent: Monday, January 19, 2015 6:39 PM
> To: Richardson, Bruce
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] lib/librte_ether: change socket_id passed to 
> rte_memzone_reserve
> 
> I would be happy with the original suggestion. If the ethdev data for a port 
> in use is in cache it removes the performance concern associated the current 
> setup and my fix. The original suggestion also fixes the crash that I was 
> seeing because of memory being reserved from a numa node with no 
> "--socket-mem" allocated.
> 
> Cian
> 
> -----Original Message-----
> From: Richardson, Bruce
> Sent: Wednesday, January 14, 2015 10:10 AM
> To: Ferriter, Cian
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] lib/librte_ether: change socket_id passed to 
> rte_memzone_reserve
> 
> On Tue, Jan 13, 2015 at 06:05:25PM +0000, Ferriter, Cian wrote:
> > Comments on alternative solutions:
> > 1) how would this solution work when there is no NIC present, and 
> > "rte_eth_from_rings" is called? Here, could you have an else where the 
> > socket id of the master core is passed to the "memzone_reserve"?
> > 2) how would you advise making this change? I have looked at where 
> > "rte_eth_dev_allocate" is being called and in all but one case, there is a 
> > "numa_id" that could be passed in. This isn't the case for " 
> > rte_eth_dev_init" however, is there an easy solution for this? Would there 
> > now need to be an "rte_eth_dev_data" struct for each socket that there is a 
> > NIC attached to, reserving memory from that socket?
> > 
> > Cian
> 
> While I think the issues you highlight can probably be overcome, I'm not so 
> sure any more how much it matters what numa node this is allocated on. The 
> ethdev data for any port in use by a port should be in the cache. In that 
> case, if it doesn't matter, your original suggestion would work fine.
> 
>       /Bruce
> 
> > 
> > -----Original Message-----
> > From: Richardson, Bruce
> > Sent: Tuesday, January 13, 2015 1:56 PM
> > To: Ferriter, Cian
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] lib/librte_ether: change socket_id 
> > passed to rte_memzone_reserve
> > 
> > On Tue, Jan 13, 2015 at 09:23:16AM +0000, Ferriter, Cian wrote:
> > > Passing a socket id of "rte_socket_id()" can cause problems in non DPDK 
> > > applications as there is a dependency on the current logical core we are 
> > > running on.
> > > Passing " rte_lcore_to_socket_id(rte_get_master_lcore())" as the socket 
> > > id to rte_memzone_reserve resolves these issues as the master lcore 
> > > doesn't change.
> > > 
> > 
> > The only trouble is that when affinitizing the memory for the NICs to the 
> > socket of the master lcore, it gives us no way to correctly configure an 
> > app to use NICs connected to two different sockets on the one system. All 
> > memory for all NICs will end up on the same socket. Two possible 
> > alternative solutions:
> > 1) affinitize memory to the socket the NIC is connected to
> > 2) add a socket parameter to the API calls to allow the user complete 
> > control over their memory allocations
> > 
> > Obviously the second one breaks backward compatibility (assume we modify 
> > existing API call), but is more powerful.
> > 
> > Thoughts?
> > 
> > /Bruce
> > 
> > > -----Original Message-----
> > > From: Ferriter, Cian
> > > Sent: Tuesday, January 13, 2015 9:22 AM
> > > To: dev at dpdk.org
> > > Cc: Ferriter, Cian
> > > Subject: [PATCH] lib/librte_ether: change socket_id passed to 
> > > rte_memzone_reserve
> > > 
> > > Change the socket id that is passed to rte_memzone_reserve from the 
> > > socket id of current logical core to the socket id of the master_lcore.
> > > ---
> > >  lib/librte_ether/rte_ethdev.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)  mode change
> > > 100644 => 100755 lib/librte_ether/rte_ethdev.c
> > > 
> > > diff --git a/lib/librte_ether/rte_ethdev.c 
> > > b/lib/librte_ether/rte_ethdev.c old mode 100644 new mode 100755 
> > > index 95f2ceb..835540d
> > > --- a/lib/librte_ether/rte_ethdev.c
> > > +++ b/lib/librte_ether/rte_ethdev.c
> > > @@ -184,7 +184,7 @@ rte_eth_dev_data_alloc(void)
> > >   if (rte_eal_process_type() == RTE_PROC_PRIMARY){
> > >           mz = rte_memzone_reserve(MZ_RTE_ETH_DEV_DATA,
> > >                           RTE_MAX_ETHPORTS * sizeof(*rte_eth_dev_data),
> > > -                         rte_socket_id(), flags);
> > > +                         rte_lcore_to_socket_id(rte_get_master_lcore()), 
> > > flags);
> > >   } else
> > >           mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
> > >   if (mz == NULL)
> > > --
> > > 1.7.4.1
> > > 

Reply via email to