Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1529094

Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible

Show all headers | View raw

On Thu, 24 Nov 2016 10:00:36 +0100
Arnd Bergmann <arnd@arndb.de> wrote:

> On Thursday, November 24, 2016 4:37:36 PM CET Jisheng Zhang wrote:
> > solB (a SW shadow cookie) perhaps gives a better performance: in hot path,
> > such as mvneta_rx(), the driver accesses buf_cookie and buf_phys_addr of
> > rx_desc which is allocated by dma_alloc_coherent, it's noncacheable if the
> > device isn't cache-coherent. I didn't measure the performance difference,
> > because in fact we take solA as well internally. From your experience,
> > can the performance gain deserve the complex code?  
> 
> Yes, a read from uncached memory is fairly slow, so if you have a chance
> to avoid that it will probably help. When adding complexity to the code,
> it probably makes sense to take a runtime profile anyway quantify how
> much it gains.
> 
> On machines that have cache-coherent DMA, accessing the descriptor
> should be fine, as you already have to load the entire cache line
> to read the status field.
> 
> Looking at this snippet:
> 
>                 rx_status = rx_desc->status;
>                 rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
>                 data = (unsigned char *)rx_desc->buf_cookie;
>                 phys_addr = rx_desc->buf_phys_addr;
>                 pool_id = MVNETA_RX_GET_BM_POOL_ID(rx_desc);
>                 bm_pool = &pp->bm_priv->bm_pools[pool_id];
> 
>                 if (!mvneta_rxq_desc_is_first_last(rx_status) ||
>                     (rx_status & MVNETA_RXD_ERR_SUMMARY)) {
> err_drop_frame_ret_pool:
>                         /* Return the buffer to the pool */
>                         mvneta_bm_pool_put_bp(pp->bm_priv, bm_pool,
>                                               rx_desc->buf_phys_addr);
> err_drop_frame:
> 
> 
> I think there is more room for optimizing if you start: you read
> the status field twice (the second one in MVNETA_RX_GET_BM_POOL_ID)
> and you can cache the buf_phys_addr along with the virtual address
> once you add that.

oh, yeah! buf_phy_addr could be included too.

> 
> Generally speaking, I'd recommend using READ_ONCE()/WRITE_ONCE()
> to access the descriptor fields, to ensure the compiler doesn't
> add extra references as well as to annotate the expensive
> operations.
> 
> 	Arnd

Got it. Thanks so much for the detailed guide. 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH net-next 0/4] Extend mvneta to support Armada 3700 (ARM 64) Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-11-22 18:00 +0100
  [PATCH net-next 4/4] ARM64: dts: marvell: Add network support for Armada 3700 Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-11-22 18:00 +0100
  [PATCH net-next 2/4] net: mvneta: Only disable mvneta_bm for 64-bits Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-11-22 18:00 +0100
  [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-11-22 18:00 +0100
    Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Arnd Bergmann <arnd@arndb.de> - 2016-11-22 22:10 +0100
      Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits  compatible Jisheng Zhang <jszhang@marvell.com> - 2016-11-23 11:10 +0100
        Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Arnd Bergmann <arnd@arndb.de> - 2016-11-23 11:20 +0100
          Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits  compatible Jisheng Zhang <jszhang@marvell.com> - 2016-11-23 12:10 +0100
          Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-11-23 14:10 +0100
            Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Marcin Wojtas <mw@semihalf.com> - 2016-11-23 17:20 +0100
              Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits  compatible Jisheng Zhang <jszhang@marvell.com> - 2016-11-24 09:50 +0100
                Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Arnd Bergmann <arnd@arndb.de> - 2016-11-24 10:10 +0100
                Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits  compatible Jisheng Zhang <jszhang@marvell.com> - 2016-11-24 10:20 +0100
                Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Marcin Wojtas <mw@semihalf.com> - 2016-11-24 16:10 +0100
                Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-11-24 16:10 +0100
                Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits  compatible Florian Fainelli <f.fainelli@gmail.com> - 2016-11-24 20:10 +0100

csiph-web