public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail•com>
To: Eric Dumazet <eric.dumazet@gmail•com>
Cc: Tariq Toukan <ttoukan.linux@gmail•com>,
	Eric Dumazet <edumazet@google•com>,
	"David S . Miller" <davem@davemloft•net>,
	netdev <netdev@vger•kernel.org>,
	Tariq Toukan <tariqt@mellanox•com>,
	Martin KaFai Lau <kafai@fb•com>,
	Willem de Bruijn <willemb@google•com>,
	Jesper Dangaard Brouer <brouer@redhat•com>,
	Brenden Blanco <bblanco@plumgrid•com>,
	Alexei Starovoitov <ast@kernel•org>
Subject: Re: [PATCH net-next 0/9] mlx4: order-0 allocations and page recycling
Date: Tue, 7 Feb 2017 11:05:49 -0800	[thread overview]
Message-ID: <20170207190546.GA51444@ast-mbp.thefacebook.com> (raw)
In-Reply-To: <1486484783.7793.61.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, Feb 07, 2017 at 08:26:23AM -0800, Eric Dumazet wrote:
> On Tue, 2017-02-07 at 08:06 -0800, Eric Dumazet wrote:
>

Awesome that you've started working on this. I think it's correct approach
and mlx5 should be cleaned up in similar way.
Long term we should be able to move all page alloc/free out of the drivers
completely.

> >  		/*
> >  		 * make sure we read the CQE after we read the ownership bit
> >  		 */
> >  		dma_rmb();
> > +		prefetch(frags[0].page);
> 
> Note that I would like to instead do a prefetch(frags[1].page)

yeah, this two look weird:
+               prefetch(frags[0].page);
+               va = page_address(frags[0].page) + frags[0].page_offset;

on most archs page_addres() is just math (not a load from memory),
but the result != frags[0].page, so I'm missing what are you trying to prefetch?

prefetch(frags[1].page)
is even more confusing. what will it prefetch?

btw we had a patch that was doing prefetch of 'va' of next packet
and it was very helpful. Like this:
   pref_index = (index + 1) & ring->size_mask;
   pref = ring->rx_info + (pref_index << priv->log_rx_info);
   prefetch(page_address(pref->page) + pref->page_offset);

but since you're redesigning rxing->rx_info... not sure how will it fit.

> So I will probably change how ring->rx_info is allocated
> 
> wasting all that space and forcing vmalloc() is silly :
> 
> tmp = size * roundup_pow_of_two(MLX4_EN_MAX_RX_FRAGS *
>                                 sizeof(struct mlx4_en_rx_alloc));

I think you'd still need roundup_pow_of_two otherwise priv->log_rx_info
optimization won't work.

  parent reply	other threads:[~2017-02-07 19:14 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-07  3:02 [PATCH net-next 0/9] mlx4: order-0 allocations and page recycling Eric Dumazet
2017-02-07  3:02 ` [PATCH net-next 1/9] mlx4: use __skb_fill_page_desc() Eric Dumazet
2017-02-07  3:02 ` [PATCH net-next 2/9] mlx4: dma_dir is a mlx4_en_priv attribute Eric Dumazet
2017-02-07  3:02 ` [PATCH net-next 3/9] mlx4: remove order field from mlx4_en_frag_info Eric Dumazet
2017-02-07  3:02 ` [PATCH net-next 4/9] mlx4: get rid of frag_prefix_size Eric Dumazet
2017-02-09 12:28   ` Tariq Toukan
2017-02-09 14:06     ` Eric Dumazet
2017-02-07  3:02 ` [PATCH net-next 5/9] mlx4: rx_headroom is a per port attribute Eric Dumazet
2017-02-07  3:02 ` [PATCH net-next 6/9] mlx4: reduce rx ring page_cache size Eric Dumazet
2017-02-07  3:02 ` [PATCH net-next 7/9] mlx4: removal of frag_sizes[] Eric Dumazet
2017-02-07  3:02 ` [PATCH net-next 8/9] mlx4: use order-0 pages for RX Eric Dumazet
2017-02-07  3:02 ` [PATCH net-next 9/9] mlx4: add page recycling in receive path Eric Dumazet
2017-02-07 16:20   ` Tariq Toukan
2017-02-07 16:34     ` Eric Dumazet
2017-02-08 10:27       ` Tariq Toukan
2017-02-07 15:50 ` [PATCH net-next 0/9] mlx4: order-0 allocations and page recycling Tariq Toukan
2017-02-07 16:06   ` Eric Dumazet
2017-02-07 16:26     ` Eric Dumazet
2017-02-07 16:28       ` Eric Dumazet
2017-02-07 19:05       ` Alexei Starovoitov [this message]
2017-02-07 19:18         ` Eric Dumazet
2017-02-08  9:02   ` Tariq Toukan
2017-02-08 10:29     ` Tariq Toukan
2017-02-08 15:52     ` Eric Dumazet
2017-02-09 12:00       ` Tariq Toukan
2017-02-09 13:31         ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170207190546.GA51444@ast-mbp.thefacebook.com \
    --to=alexei.starovoitov@gmail$(echo .)com \
    --cc=ast@kernel$(echo .)org \
    --cc=bblanco@plumgrid$(echo .)com \
    --cc=brouer@redhat$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=edumazet@google$(echo .)com \
    --cc=eric.dumazet@gmail$(echo .)com \
    --cc=kafai@fb$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=tariqt@mellanox$(echo .)com \
    --cc=ttoukan.linux@gmail$(echo .)com \
    --cc=willemb@google$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox