From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Gerhard Engleder <gerhard@engleder-embedded.com>
Cc: <netdev@vger.kernel.org>, <bpf@vger.kernel.org>,
<davem@davemloft.net>, <kuba@kernel.org>, <edumazet@google.com>,
<pabeni@redhat.com>, <bjorn@kernel.org>,
<magnus.karlsson@intel.com>, <jonathan.lemon@gmail.com>
Subject: Re: [PATCH net-next v3 1/6] tsnep: Replace modulo operation with mask
Date: Thu, 20 Apr 2023 16:23:10 +0200 [thread overview]
Message-ID: <ZEFKzuPKGRv0bO35@boxer> (raw)
In-Reply-To: <20230418190459.19326-2-gerhard@engleder-embedded.com>
On Tue, Apr 18, 2023 at 09:04:54PM +0200, Gerhard Engleder wrote:
> TX/RX ring size is static and power of 2 to enable compiler to optimize
> modulo operation to mask operation. Make this optimization already in
> the code and don't rely on the compiler.
I think this came out of my review, so:
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Does this give you a minor perf boost?
>
> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> ---
> drivers/net/ethernet/engleder/tsnep.h | 1 +
> drivers/net/ethernet/engleder/tsnep_main.c | 28 +++++++++++-----------
> 2 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/engleder/tsnep.h b/drivers/net/ethernet/engleder/tsnep.h
> index 058c2bcf31a7..1de26aec78d3 100644
> --- a/drivers/net/ethernet/engleder/tsnep.h
> +++ b/drivers/net/ethernet/engleder/tsnep.h
> @@ -18,6 +18,7 @@
> #define TSNEP "tsnep"
>
> #define TSNEP_RING_SIZE 256
> +#define TSNEP_RING_MASK (TSNEP_RING_SIZE - 1)
> #define TSNEP_RING_RX_REFILL 16
> #define TSNEP_RING_RX_REUSE (TSNEP_RING_SIZE - TSNEP_RING_SIZE / 4)
> #define TSNEP_RING_ENTRIES_PER_PAGE (PAGE_SIZE / TSNEP_DESC_SIZE)
> diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
> index ed1b6102cfeb..3d15e673894a 100644
> --- a/drivers/net/ethernet/engleder/tsnep_main.c
> +++ b/drivers/net/ethernet/engleder/tsnep_main.c
> @@ -292,7 +292,7 @@ static int tsnep_tx_ring_init(struct tsnep_tx *tx)
> }
> for (i = 0; i < TSNEP_RING_SIZE; i++) {
> entry = &tx->entry[i];
> - next_entry = &tx->entry[(i + 1) % TSNEP_RING_SIZE];
> + next_entry = &tx->entry[(i + 1) & TSNEP_RING_MASK];
> entry->desc->next = __cpu_to_le64(next_entry->desc_dma);
> }
>
> @@ -381,7 +381,7 @@ static int tsnep_tx_map(struct sk_buff *skb, struct tsnep_tx *tx, int count)
> int i;
>
> for (i = 0; i < count; i++) {
> - entry = &tx->entry[(tx->write + i) % TSNEP_RING_SIZE];
> + entry = &tx->entry[(tx->write + i) & TSNEP_RING_MASK];
>
> if (!i) {
> len = skb_headlen(skb);
> @@ -419,7 +419,7 @@ static int tsnep_tx_unmap(struct tsnep_tx *tx, int index, int count)
> int i;
>
> for (i = 0; i < count; i++) {
> - entry = &tx->entry[(index + i) % TSNEP_RING_SIZE];
> + entry = &tx->entry[(index + i) & TSNEP_RING_MASK];
>
> if (entry->len) {
> if (entry->type & TSNEP_TX_TYPE_SKB)
> @@ -481,9 +481,9 @@ static netdev_tx_t tsnep_xmit_frame_ring(struct sk_buff *skb,
> skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
>
> for (i = 0; i < count; i++)
> - tsnep_tx_activate(tx, (tx->write + i) % TSNEP_RING_SIZE, length,
> + tsnep_tx_activate(tx, (tx->write + i) & TSNEP_RING_MASK, length,
> i == count - 1);
> - tx->write = (tx->write + count) % TSNEP_RING_SIZE;
> + tx->write = (tx->write + count) & TSNEP_RING_MASK;
>
> skb_tx_timestamp(skb);
>
> @@ -516,7 +516,7 @@ static int tsnep_xdp_tx_map(struct xdp_frame *xdpf, struct tsnep_tx *tx,
> frag = NULL;
> len = xdpf->len;
> for (i = 0; i < count; i++) {
> - entry = &tx->entry[(tx->write + i) % TSNEP_RING_SIZE];
> + entry = &tx->entry[(tx->write + i) & TSNEP_RING_MASK];
> if (type & TSNEP_TX_TYPE_XDP_NDO) {
> data = unlikely(frag) ? skb_frag_address(frag) :
> xdpf->data;
> @@ -589,9 +589,9 @@ static bool tsnep_xdp_xmit_frame_ring(struct xdp_frame *xdpf,
> length = retval;
>
> for (i = 0; i < count; i++)
> - tsnep_tx_activate(tx, (tx->write + i) % TSNEP_RING_SIZE, length,
> + tsnep_tx_activate(tx, (tx->write + i) & TSNEP_RING_MASK, length,
> i == count - 1);
> - tx->write = (tx->write + count) % TSNEP_RING_SIZE;
> + tx->write = (tx->write + count) & TSNEP_RING_MASK;
>
> /* descriptor properties shall be valid before hardware is notified */
> dma_wmb();
> @@ -691,7 +691,7 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
> /* xdpf is union with skb */
> entry->skb = NULL;
>
> - tx->read = (tx->read + count) % TSNEP_RING_SIZE;
> + tx->read = (tx->read + count) & TSNEP_RING_MASK;
>
> tx->packets++;
> tx->bytes += length + ETH_FCS_LEN;
> @@ -839,7 +839,7 @@ static int tsnep_rx_ring_init(struct tsnep_rx *rx)
>
> for (i = 0; i < TSNEP_RING_SIZE; i++) {
> entry = &rx->entry[i];
> - next_entry = &rx->entry[(i + 1) % TSNEP_RING_SIZE];
> + next_entry = &rx->entry[(i + 1) & TSNEP_RING_MASK];
> entry->desc->next = __cpu_to_le64(next_entry->desc_dma);
> }
>
> @@ -925,7 +925,7 @@ static int tsnep_rx_refill(struct tsnep_rx *rx, int count, bool reuse)
> int retval;
>
> for (i = 0; i < count && !alloc_failed; i++) {
> - index = (rx->write + i) % TSNEP_RING_SIZE;
> + index = (rx->write + i) & TSNEP_RING_MASK;
>
> retval = tsnep_rx_alloc_buffer(rx, index);
> if (unlikely(retval)) {
> @@ -945,7 +945,7 @@ static int tsnep_rx_refill(struct tsnep_rx *rx, int count, bool reuse)
> }
>
> if (enable) {
> - rx->write = (rx->write + i) % TSNEP_RING_SIZE;
> + rx->write = (rx->write + i) & TSNEP_RING_MASK;
>
> /* descriptor properties shall be valid before hardware is
> * notified
> @@ -1090,7 +1090,7 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi,
> * empty RX ring, thus buffer cannot be used for
> * RX processing
> */
> - rx->read = (rx->read + 1) % TSNEP_RING_SIZE;
> + rx->read = (rx->read + 1) & TSNEP_RING_MASK;
> desc_available++;
>
> rx->dropped++;
> @@ -1117,7 +1117,7 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi,
> */
> length -= TSNEP_RX_INLINE_METADATA_SIZE;
>
> - rx->read = (rx->read + 1) % TSNEP_RING_SIZE;
> + rx->read = (rx->read + 1) & TSNEP_RING_MASK;
> desc_available++;
>
> if (prog) {
> --
> 2.30.2
>
next prev parent reply other threads:[~2023-04-20 14:23 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-18 19:04 [PATCH net-next v3 0/6] tsnep: XDP socket zero-copy support Gerhard Engleder
2023-04-18 19:04 ` [PATCH net-next v3 1/6] tsnep: Replace modulo operation with mask Gerhard Engleder
2023-04-20 14:23 ` Maciej Fijalkowski [this message]
2023-04-20 15:10 ` Stephen Hemminger
2023-04-20 18:51 ` Gerhard Engleder
2023-04-20 18:40 ` Gerhard Engleder
2023-04-18 19:04 ` [PATCH net-next v3 2/6] tsnep: Rework TX/RX queue initialization Gerhard Engleder
2023-04-18 19:04 ` [PATCH net-next v3 3/6] tsnep: Add functions for queue enable/disable Gerhard Engleder
2023-04-18 19:04 ` [PATCH net-next v3 4/6] tsnep: Move skb receive action to separate function Gerhard Engleder
2023-04-18 19:04 ` [PATCH net-next v3 5/6] tsnep: Add XDP socket zero-copy RX support Gerhard Engleder
2023-04-20 19:46 ` Maciej Fijalkowski
2023-04-21 18:54 ` Gerhard Engleder
2023-04-18 19:04 ` [PATCH net-next v3 6/6] tsnep: Add XDP socket zero-copy TX support Gerhard Engleder
2023-04-20 20:17 ` Maciej Fijalkowski
2023-04-21 19:02 ` Gerhard Engleder
2023-04-24 12:03 ` Maciej Fijalkowski
2023-04-24 18:40 ` Gerhard Engleder
2023-04-20 20:19 ` [PATCH net-next v3 0/6] tsnep: XDP socket zero-copy support Maciej Fijalkowski
2023-04-21 19:05 ` Gerhard Engleder
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=ZEFKzuPKGRv0bO35@boxer \
--to=maciej.fijalkowski@intel.com \
--cc=bjorn@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gerhard@engleder-embedded.com \
--cc=jonathan.lemon@gmail.com \
--cc=kuba@kernel.org \
--cc=magnus.karlsson@intel.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).