From: Bimmy Pujari <bimmy.pujari@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [next PATCH S52-V2 10/15] i40e: simplify txd use count calculation
Date: Tue, 8 Nov 2016 13:05:14 -0800 [thread overview]
Message-ID: <1478639119-14656-11-git-send-email-bimmy.pujari@intel.com> (raw)
In-Reply-To: <1478639119-14656-1-git-send-email-bimmy.pujari@intel.com>
From: Mitch Williams <mitch.a.williams@intel.com>
The i40e_txd_use_count function was fast but confusing. In the comments,
it even admits that it's ugly. So replace it with a new function that is
(very) slightly faster and has extensive commenting to help the thicker
among us (including the author, who will forget in a week) understand
how it works.
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Change-ID: Ifb533f13786a0bf39cb29f77969a5be2c83d9a87
---
Testing Hints : Lots and lots of TSO with various
data sizes. This should operate exactly as before, with no noticeable
performance change.
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 45 +++++++++++++++++----------
drivers/net/ethernet/intel/i40evf/i40e_txrx.h | 45 +++++++++++++++++----------
2 files changed, 56 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index de8550f..e065321 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -173,26 +173,37 @@ static inline bool i40e_test_staterr(union i40e_rx_desc *rx_desc,
#define I40E_MAX_DATA_PER_TXD_ALIGNED \
(I40E_MAX_DATA_PER_TXD & ~(I40E_MAX_READ_REQ_SIZE - 1))
-/* This ugly bit of math is equivalent to DIV_ROUNDUP(size, X) where X is
- * the value I40E_MAX_DATA_PER_TXD_ALIGNED. It is needed due to the fact
- * that 12K is not a power of 2 and division is expensive. It is used to
- * approximate the number of descriptors used per linear buffer. Note
- * that this will overestimate in some cases as it doesn't account for the
- * fact that we will add up to 4K - 1 in aligning the 12K buffer, however
- * the error should not impact things much as large buffers usually mean
- * we will use fewer descriptors then there are frags in an skb.
+/**
+ * i40e_txd_use_count - estimate the number of descriptors needed for Tx
+ * @size: transmit request size in bytes
+ *
+ * Due to hardware alignment restrictions (4K alignment), we need to
+ * assume that we can have no more than 12K of data per descriptor, even
+ * though each descriptor can take up to 16K - 1 bytes of aligned memory.
+ * Thus, we need to divide by 12K. But division is slow! Instead,
+ * we decompose the operation into shifts and one relatively cheap
+ * multiply operation.
+ *
+ * To divide by 12K, we first divide by 4K, then divide by 3:
+ * To divide by 4K, shift right by 12 bits
+ * To divide by 3, multiply by 85, then divide by 256
+ * (Divide by 256 is done by shifting right by 8 bits)
+ * Finally, we add one to round up. Because 256 isn't an exact multiple of
+ * 3, we'll underestimate near each multiple of 12K. This is actually more
+ * accurate as we have 4K - 1 of wiggle room that we can fit into the last
+ * segment. For our purposes this is accurate out to 1M which is orders of
+ * magnitude greater than our largest possible GSO size.
+ *
+ * This would then be implemented as:
+ * return (((size >> 12) * 85) >> 8) + 1;
+ *
+ * Since multiplication and division are commutative, we can reorder
+ * operations into:
+ * return ((size * 85) >> 20) + 1;
*/
static inline unsigned int i40e_txd_use_count(unsigned int size)
{
- const unsigned int max = I40E_MAX_DATA_PER_TXD_ALIGNED;
- const unsigned int reciprocal = ((1ull << 32) - 1 + (max / 2)) / max;
- unsigned int adjust = ~(u32)0;
-
- /* if we rounded up on the reciprocal pull down the adjustment */
- if ((max * reciprocal) > adjust)
- adjust = ~(u32)(reciprocal - 1);
-
- return (u32)((((u64)size * reciprocal) + adjust) >> 32);
+ return ((size * 85) >> 20) + 1;
}
/* Tx Descriptors needed, worst case */
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
index a586e19..a5fc789 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
@@ -173,26 +173,37 @@ static inline bool i40e_test_staterr(union i40e_rx_desc *rx_desc,
#define I40E_MAX_DATA_PER_TXD_ALIGNED \
(I40E_MAX_DATA_PER_TXD & ~(I40E_MAX_READ_REQ_SIZE - 1))
-/* This ugly bit of math is equivalent to DIV_ROUNDUP(size, X) where X is
- * the value I40E_MAX_DATA_PER_TXD_ALIGNED. It is needed due to the fact
- * that 12K is not a power of 2 and division is expensive. It is used to
- * approximate the number of descriptors used per linear buffer. Note
- * that this will overestimate in some cases as it doesn't account for the
- * fact that we will add up to 4K - 1 in aligning the 12K buffer, however
- * the error should not impact things much as large buffers usually mean
- * we will use fewer descriptors then there are frags in an skb.
+/**
+ * i40e_txd_use_count - estimate the number of descriptors needed for Tx
+ * @size: transmit request size in bytes
+ *
+ * Due to hardware alignment restrictions (4K alignment), we need to
+ * assume that we can have no more than 12K of data per descriptor, even
+ * though each descriptor can take up to 16K - 1 bytes of aligned memory.
+ * Thus, we need to divide by 12K. But division is slow! Instead,
+ * we decompose the operation into shifts and one relatively cheap
+ * multiply operation.
+ *
+ * To divide by 12K, we first divide by 4K, then divide by 3:
+ * To divide by 4K, shift right by 12 bits
+ * To divide by 3, multiply by 85, then divide by 256
+ * (Divide by 256 is done by shifting right by 8 bits)
+ * Finally, we add one to round up. Because 256 isn't an exact multiple of
+ * 3, we'll underestimate near each multiple of 12K. This is actually more
+ * accurate as we have 4K - 1 of wiggle room that we can fit into the last
+ * segment. For our purposes this is accurate out to 1M which is orders of
+ * magnitude greater than our largest possible GSO size.
+ *
+ * This would then be implemented as:
+ * return (((size >> 12) * 85) >> 8) + 1;
+ *
+ * Since multiplication and division are commutative, we can reorder
+ * operations into:
+ * return ((size * 85) >> 20) + 1;
*/
static inline unsigned int i40e_txd_use_count(unsigned int size)
{
- const unsigned int max = I40E_MAX_DATA_PER_TXD_ALIGNED;
- const unsigned int reciprocal = ((1ull << 32) - 1 + (max / 2)) / max;
- unsigned int adjust = ~(u32)0;
-
- /* if we rounded up on the reciprocal pull down the adjustment */
- if ((max * reciprocal) > adjust)
- adjust = ~(u32)(reciprocal - 1);
-
- return (u32)((((u64)size * reciprocal) + adjust) >> 32);
+ return ((size * 85) >> 20) + 1;
}
/* Tx Descriptors needed, worst case */
--
2.4.11
next prev parent reply other threads:[~2016-11-08 21:05 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-08 21:05 [Intel-wired-lan] [next PATCH S52-V2 00/15] i40e/i40evf updates Bimmy Pujari
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 01/15] i40evf: check for msix_entries null dereference Bimmy Pujari
2016-11-16 18:56 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 02/15] i40e: Remove unreachable code Bimmy Pujari
2016-11-16 18:58 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 03/15] i40e: Pass unknown PHY type for unknown PHYs Bimmy Pujari
2016-11-16 19:21 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 04/15] i40evf: protect against NULL msix_entries and q_vectors pointers Bimmy Pujari
2016-11-16 19:24 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 05/15] i40e: Fix for ethtool Supported link modes Bimmy Pujari
2016-11-16 19:27 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 06/15] i40e: set broadcast promiscuous mode for each active VLAN Bimmy Pujari
2016-11-21 21:17 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 07/15] i40e/i40evf: replace for memcpy with single memcpy call in ethtool Bimmy Pujari
2016-11-16 20:34 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 08/15] i40e: change message to only appear when extra debug info is wanted Bimmy Pujari
2016-11-16 20:39 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 09/15] i40e: Driver prints log message on link speed change Bimmy Pujari
2016-11-16 23:51 ` Bowers, AndrewX
2016-11-08 21:05 ` Bimmy Pujari [this message]
2016-11-17 18:06 ` [Intel-wired-lan] [next PATCH S52-V2 10/15] i40e: simplify txd use count calculation Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 11/15] i40e: restore workaround for removing default MAC filter Bimmy Pujari
2016-11-17 18:11 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 12/15] i40e/i40evf: napi_poll must return the work done Bimmy Pujari
2016-11-17 18:27 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 13/15] i40e: remove code to handle dev_addr specially Bimmy Pujari
2016-11-17 18:33 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 14/15] i40e: Blink LED on 1G BaseT boards Bimmy Pujari
2016-11-21 20:00 ` Bowers, AndrewX
2016-11-08 21:05 ` [Intel-wired-lan] [next PATCH S52-V2 15/15] Changed version from 1.6.21 to 1.6.25 Bimmy Pujari
2016-11-17 18:36 ` Bowers, AndrewX
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=1478639119-14656-11-git-send-email-bimmy.pujari@intel.com \
--to=bimmy.pujari@intel.com \
--cc=intel-wired-lan@osuosl.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.