From: Arnd Bergmann <arnd@arndb.de>
To: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
"Alan Brady" <alan.brady@intel.com>,
"Jacob Keller" <jacob.e.keller@intel.com>,
"Amritha Nambiar" <amritha.nambiar@intel.com>,
"Mitch Williams" <mitch.a.williams@intel.com>,
"Alexander Duyck" <alexander.h.duyck@intel.com>,
"Filip Sadowski" <filip.sadowski@intel.com>,
"David S. Miller" <davem@davemloft.net>,
"Björn Töpel" <bjorn.topel@intel.com>,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH] [v2] i40e: avoid 64-bit division where possible
Date: Tue, 17 Oct 2017 17:49:48 +0200 [thread overview]
Message-ID: <20171017155001.318184-1-arnd@arndb.de> (raw)
The new bandwidth calculation caused a link error on 32-bit
architectures, like
ERROR: "__aeabi_uldivmod" [drivers/net/ethernet/intel/i40e/i40e.ko] undefined!
The problem is the max_tx_rate calculation that uses 64-bit integers.
This is not really necessary since the numbers are in MBit/s so
they won't be higher than 40000 for the highest support rate, and
are guaranteed to not exceed 2^32 in future generations either.
Another patch from Alan Brady fixed the link error by adding
many calls to do_div(), which makes the code less efficent and
less readable than necessary.
This changes the representation to 'u32' when dealing with MBit/s
and uses div_u64() to convert from u64 numbers in byte/s, reverting
parts of Alan's earlier fix that have become obsolete now.
Fixes: 2027d4deacb1 ("i40e: Add support setting TC max bandwidth rates")
Fixes: 73983b5ae011 ("i40e: fix u64 division usage")
Cc: Alan Brady <alan.brady@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/intel/i40e/i40e.h | 4 +-
drivers/net/ethernet/intel/i40e/i40e_main.c | 70 +++++++++++------------------
2 files changed, 27 insertions(+), 47 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index c3f13120f3ce..c7aa0c982273 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -407,7 +407,7 @@ struct i40e_channel {
u8 enabled_tc;
struct i40e_aqc_vsi_properties_data info;
- u64 max_tx_rate;
+ u32 max_tx_rate; /* in Mbits/s */
/* track this channel belongs to which VSI */
struct i40e_vsi *parent_vsi;
@@ -1101,5 +1101,5 @@ static inline bool i40e_enabled_xdp_vsi(struct i40e_vsi *vsi)
}
int i40e_create_queue_channel(struct i40e_vsi *vsi, struct i40e_channel *ch);
-int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate);
+int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u32 max_tx_rate);
#endif /* _I40E_H_ */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 3ceda140170d..57682cc78508 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5448,17 +5448,16 @@ int i40e_get_link_speed(struct i40e_vsi *vsi)
*
* Helper function to set BW limit for a given VSI
**/
-int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate)
+int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u32 max_tx_rate)
{
struct i40e_pf *pf = vsi->back;
- u64 credits = 0;
int speed = 0;
int ret = 0;
speed = i40e_get_link_speed(vsi);
if (max_tx_rate > speed) {
dev_err(&pf->pdev->dev,
- "Invalid max tx rate %llu specified for VSI seid %d.",
+ "Invalid max tx rate %u specified for VSI seid %d.",
max_tx_rate, seid);
return -EINVAL;
}
@@ -5469,13 +5468,12 @@ int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate)
}
/* Tx rate credits are in values of 50Mbps, 0 is disabled */
- credits = max_tx_rate;
- do_div(credits, I40E_BW_CREDIT_DIVISOR);
- ret = i40e_aq_config_vsi_bw_limit(&pf->hw, seid, credits,
+ ret = i40e_aq_config_vsi_bw_limit(&pf->hw, seid,
+ max_tx_rate / I40E_BW_CREDIT_DIVISOR,
I40E_MAX_BW_INACTIVE_ACCUM, NULL);
if (ret)
dev_err(&pf->pdev->dev,
- "Failed set tx rate (%llu Mbps) for vsi->seid %u, err %s aq_err %s\n",
+ "Failed set tx rate (%u Mbps) for vsi->seid %u, err %s aq_err %s\n",
max_tx_rate, seid, i40e_stat_str(&pf->hw, ret),
i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));
return ret;
@@ -6158,17 +6156,13 @@ int i40e_create_queue_channel(struct i40e_vsi *vsi,
/* configure VSI for BW limit */
if (ch->max_tx_rate) {
- u64 credits = ch->max_tx_rate;
-
if (i40e_set_bw_limit(vsi, ch->seid, ch->max_tx_rate))
return -EINVAL;
- do_div(credits, I40E_BW_CREDIT_DIVISOR);
dev_dbg(&pf->pdev->dev,
- "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
+ "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
ch->max_tx_rate,
- credits,
- ch->seid);
+ ch->max_tx_rate / I40E_BW_CREDIT_DIVISOR, ch->seid);
}
/* in case of VF, this will be main SRIOV VSI */
@@ -6189,7 +6183,6 @@ int i40e_create_queue_channel(struct i40e_vsi *vsi,
static int i40e_configure_queue_channels(struct i40e_vsi *vsi)
{
struct i40e_channel *ch;
- u64 max_rate = 0;
int ret = 0, i;
/* Create app vsi with the TCs. Main VSI with TC0 is already set up */
@@ -6211,9 +6204,8 @@ static int i40e_configure_queue_channels(struct i40e_vsi *vsi)
/* Bandwidth limit through tc interface is in bytes/s,
* change to Mbit/s
*/
- max_rate = vsi->mqprio_qopt.max_rate[i];
- do_div(max_rate, I40E_BW_MBPS_DIVISOR);
- ch->max_tx_rate = max_rate;
+ ch->max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[i],
+ I40E_BW_CREDIT_DIVISOR);
list_add_tail(&ch->list, &vsi->ch_list);
@@ -6643,7 +6635,6 @@ static int i40e_validate_mqprio_qopt(struct i40e_vsi *vsi,
struct tc_mqprio_qopt_offload *mqprio_qopt)
{
u64 sum_max_rate = 0;
- u64 max_rate = 0;
int i;
if (mqprio_qopt->qopt.offset[0] != 0 ||
@@ -6658,9 +6649,8 @@ static int i40e_validate_mqprio_qopt(struct i40e_vsi *vsi,
"Invalid min tx rate (greater than 0) specified\n");
return -EINVAL;
}
- max_rate = mqprio_qopt->max_rate[i];
- do_div(max_rate, I40E_BW_MBPS_DIVISOR);
- sum_max_rate += max_rate;
+ sum_max_rate += div_u64(mqprio_qopt->max_rate[i],
+ I40E_BW_CREDIT_DIVISOR);
if (i >= mqprio_qopt->qopt.num_tc - 1)
break;
@@ -6804,18 +6794,14 @@ static int i40e_setup_tc(struct net_device *netdev, void *type_data)
if (pf->flags & I40E_FLAG_TC_MQPRIO) {
if (vsi->mqprio_qopt.max_rate[0]) {
- u64 max_tx_rate = vsi->mqprio_qopt.max_rate[0];
-
- do_div(max_tx_rate, I40E_BW_MBPS_DIVISOR);
+ u32 max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[0],
+ I40E_BW_CREDIT_DIVISOR);
ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
if (!ret) {
- u64 credits = max_tx_rate;
-
- do_div(credits, I40E_BW_CREDIT_DIVISOR);
dev_dbg(&vsi->back->pdev->dev,
- "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
+ "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
max_tx_rate,
- credits,
+ max_tx_rate / I40E_BW_CREDIT_DIVISOR,
vsi->seid);
} else {
need_reset = true;
@@ -9024,15 +9010,12 @@ static int i40e_rebuild_channels(struct i40e_vsi *vsi)
return ret;
}
if (ch->max_tx_rate) {
- u64 credits = ch->max_tx_rate;
-
if (i40e_set_bw_limit(vsi, ch->seid,
ch->max_tx_rate))
return -EINVAL;
- do_div(credits, I40E_BW_CREDIT_DIVISOR);
dev_dbg(&vsi->back->pdev->dev,
- "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
+ "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
ch->max_tx_rate,
ch->max_tx_rate / I40E_BW_CREDIT_DIVISOR,
ch->seid);
@@ -9314,21 +9297,18 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
}
if (vsi->mqprio_qopt.max_rate[0]) {
- u64 max_tx_rate = vsi->mqprio_qopt.max_rate[0];
- u64 credits = 0;
+ u32 max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[0],
+ I40E_BW_CREDIT_DIVISOR);
- do_div(max_tx_rate, I40E_BW_MBPS_DIVISOR);
ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
- if (ret)
+ if (!ret)
+ dev_dbg(&vsi->back->pdev->dev,
+ "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
+ max_tx_rate,
+ max_tx_rate / I40E_BW_CREDIT_DIVISOR,
+ vsi->seid);
+ else
goto end_unlock;
-
- credits = max_tx_rate;
- do_div(credits, I40E_BW_CREDIT_DIVISOR);
- dev_dbg(&vsi->back->pdev->dev,
- "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
- max_tx_rate,
- credits,
- vsi->seid);
}
ret = i40e_rebuild_cloud_filters(vsi, vsi->seid);
--
2.9.0
next reply other threads:[~2017-10-17 15:50 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-17 15:49 Arnd Bergmann [this message]
2017-10-17 17:33 ` [Intel-wired-lan] " Alexander Duyck
2017-10-18 5:37 ` Nambiar, Amritha
2017-10-20 17:10 ` Alexander Duyck
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=20171017155001.318184-1-arnd@arndb.de \
--to=arnd@arndb.de \
--cc=alan.brady@intel.com \
--cc=alexander.h.duyck@intel.com \
--cc=amritha.nambiar@intel.com \
--cc=bjorn.topel@intel.com \
--cc=davem@davemloft.net \
--cc=filip.sadowski@intel.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacob.e.keller@intel.com \
--cc=jeffrey.t.kirsher@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mitch.a.williams@intel.com \
--cc=netdev@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®