This series revises the algorithm used for replenishing receive
buffers on RX endpoints. Currently there are two atomic variables
that track how many receive buffers can be sent to the hardware.
The new algorithm obviates the need for those, by just assuming we
always want to provide the hardware with buffers until it can hold
no more.
The first patch eliminates an atomic variable that's not required.
The next moves some code into the main replenish function's caller,
making one of the called function's arguments unnecessary. The
next six refactor things a bit more, adding a new helper function
that allows us to eliminate an additional atomic variable. And the
final two implement two more minor improvements.
-Alex
Alex Elder (10):
net: ipa: kill replenish_saved
net: ipa: allocate transaction before pages when replenishing
net: ipa: increment backlog in replenish caller
net: ipa: decide on doorbell in replenish loop
net: ipa: allocate transaction in replenish loop
net: ipa: don't use replenish_backlog
net: ipa: introduce gsi_channel_trans_idle()
net: ipa: kill replenish_backlog
net: ipa: replenish after delivering payload
net: ipa: determine replenish doorbell differently
drivers/net/ipa/gsi_trans.c | 11 ++++
drivers/net/ipa/gsi_trans.h | 10 +++
drivers/net/ipa/ipa_endpoint.c | 112 +++++++++++----------------------
drivers/net/ipa/ipa_endpoint.h | 8 +--
4 files changed, 60 insertions(+), 81 deletions(-)
--
2.32.0
When replenishing, have ipa_endpoint_replenish() allocate a
transaction, and pass that to ipa_endpoint_replenish_one() to fill.
Then, if that produces no error, commit the transaction within the
replenish loop as well. In this way we can distinguish between
transaction failures and buffer allocation/mapping failures.
Failure to allocate a transaction simply means the hardware already
has as many receive buffers as it can hold. In that case we can
break out of the replenish loop because there's nothing more to do.
If we fail to allocate or map pages for the receive buffer, just
try again later.
Signed-off-by: Alex Elder <[email protected]>
---
drivers/net/ipa/ipa_endpoint.c | 40 ++++++++++++++--------------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 274cf1c30b593..f5367b902c27c 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -1036,24 +1036,19 @@ static void ipa_endpoint_status(struct ipa_endpoint *endpoint)
iowrite32(val, ipa->reg_virt + offset);
}
-static int
-ipa_endpoint_replenish_one(struct ipa_endpoint *endpoint, bool doorbell)
+static int ipa_endpoint_replenish_one(struct ipa_endpoint *endpoint,
+ struct gsi_trans *trans)
{
- struct gsi_trans *trans;
struct page *page;
u32 buffer_size;
u32 offset;
u32 len;
int ret;
- trans = ipa_endpoint_trans_alloc(endpoint, 1);
- if (!trans)
- return -ENOMEM;
-
buffer_size = endpoint->data->rx.buffer_size;
page = dev_alloc_pages(get_order(buffer_size));
if (!page)
- goto err_trans_free;
+ return -ENOMEM;
/* Offset the buffer to make space for skb headroom */
offset = NET_SKB_PAD;
@@ -1061,19 +1056,11 @@ ipa_endpoint_replenish_one(struct ipa_endpoint *endpoint, bool doorbell)
ret = gsi_trans_page_add(trans, page, len, offset);
if (ret)
- goto err_free_pages;
- trans->data = page; /* transaction owns page now */
+ __free_pages(page, get_order(buffer_size));
+ else
+ trans->data = page; /* transaction owns page now */
- gsi_trans_commit(trans, doorbell);
-
- return 0;
-
-err_free_pages:
- __free_pages(page, get_order(buffer_size));
-err_trans_free:
- gsi_trans_free(trans);
-
- return -ENOMEM;
+ return ret;
}
/**
@@ -1089,6 +1076,7 @@ ipa_endpoint_replenish_one(struct ipa_endpoint *endpoint, bool doorbell)
*/
static void ipa_endpoint_replenish(struct ipa_endpoint *endpoint)
{
+ struct gsi_trans *trans;
struct gsi *gsi;
u32 backlog;
@@ -1100,15 +1088,18 @@ static void ipa_endpoint_replenish(struct ipa_endpoint *endpoint)
return;
while (atomic_dec_not_zero(&endpoint->replenish_backlog)) {
- bool doorbell;
+ trans = ipa_endpoint_trans_alloc(endpoint, 1);
+ if (!trans)
+ break;
+
+ if (ipa_endpoint_replenish_one(endpoint, trans))
+ goto try_again_later;
if (++endpoint->replenish_ready == IPA_REPLENISH_BATCH)
endpoint->replenish_ready = 0;
/* Ring the doorbell if we've got a full batch */
- doorbell = !endpoint->replenish_ready;
- if (ipa_endpoint_replenish_one(endpoint, doorbell))
- goto try_again_later;
+ gsi_trans_commit(trans, !endpoint->replenish_ready);
}
clear_bit(IPA_REPLENISH_ACTIVE, endpoint->replenish_flags);
@@ -1116,6 +1107,7 @@ static void ipa_endpoint_replenish(struct ipa_endpoint *endpoint)
return;
try_again_later:
+ gsi_trans_free(trans);
clear_bit(IPA_REPLENISH_ACTIVE, endpoint->replenish_flags);
/* The last one didn't succeed, so fix the backlog */
--
2.32.0
A transaction failure only occurs if no more transactions are
available for an endpoint. It's a very cheap test.
When replenishing an RX endpoint buffer, there's no point in
allocating pages if transactions are exhausted. So don't bother
doing so unless the transaction allocation succeeds.
Signed-off-by: Alex Elder <[email protected]>
---
drivers/net/ipa/ipa_endpoint.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index a9f6d4083f869..f8dbd43949e16 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -1046,14 +1046,14 @@ static int ipa_endpoint_replenish_one(struct ipa_endpoint *endpoint)
u32 len;
int ret;
+ trans = ipa_endpoint_trans_alloc(endpoint, 1);
+ if (!trans)
+ return -ENOMEM;
+
buffer_size = endpoint->data->rx.buffer_size;
page = dev_alloc_pages(get_order(buffer_size));
if (!page)
- return -ENOMEM;
-
- trans = ipa_endpoint_trans_alloc(endpoint, 1);
- if (!trans)
- goto err_free_pages;
+ goto err_trans_free;
/* Offset the buffer to make space for skb headroom */
offset = NET_SKB_PAD;
@@ -1061,7 +1061,7 @@ static int ipa_endpoint_replenish_one(struct ipa_endpoint *endpoint)
ret = gsi_trans_page_add(trans, page, len, offset);
if (ret)
- goto err_trans_free;
+ goto err_free_pages;
trans->data = page; /* transaction owns page now */
if (++endpoint->replenish_ready == IPA_REPLENISH_BATCH) {
@@ -1073,10 +1073,10 @@ static int ipa_endpoint_replenish_one(struct ipa_endpoint *endpoint)
return 0;
-err_trans_free:
- gsi_trans_free(trans);
err_free_pages:
__free_pages(page, get_order(buffer_size));
+err_trans_free:
+ gsi_trans_free(trans);
return -ENOMEM;
}
--
2.32.0
The replenish_saved field keeps track of the number of times a new
buffer is added to the backlog when replenishing is disabled. We
don't really use it though, so there's no need for us to track it
separately. Whether replenishing is enabled or not, we can simply
increment the backlog.
Get rid of replenish_saved, and initialize and increment the backlog
where it would have otherwise been used.
Signed-off-by: Alex Elder <[email protected]>
---
drivers/net/ipa/ipa_endpoint.c | 17 ++++-------------
drivers/net/ipa/ipa_endpoint.h | 2 --
2 files changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index fffd0a784ef2c..a9f6d4083f869 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -1090,9 +1090,8 @@ static int ipa_endpoint_replenish_one(struct ipa_endpoint *endpoint)
* endpoint, based on the number of entries in the underlying channel ring
* buffer. If an endpoint's "backlog" is non-zero, it indicates how many
* more receive buffers can be supplied to the hardware. Replenishing for
- * an endpoint can be disabled, in which case requests to replenish a
- * buffer are "saved", and transferred to the backlog once it is re-enabled
- * again.
+ * an endpoint can be disabled, in which case buffers are not queued to
+ * the hardware.
*/
static void ipa_endpoint_replenish(struct ipa_endpoint *endpoint, bool add_one)
{
@@ -1102,7 +1101,7 @@ static void ipa_endpoint_replenish(struct ipa_endpoint *endpoint, bool add_one)
if (!test_bit(IPA_REPLENISH_ENABLED, endpoint->replenish_flags)) {
if (add_one)
- atomic_inc(&endpoint->replenish_saved);
+ atomic_inc(&endpoint->replenish_backlog);
return;
}
@@ -1147,11 +1146,8 @@ static void ipa_endpoint_replenish_enable(struct ipa_endpoint *endpoint)
{
struct gsi *gsi = &endpoint->ipa->gsi;
u32 max_backlog;
- u32 saved;
set_bit(IPA_REPLENISH_ENABLED, endpoint->replenish_flags);
- while ((saved = atomic_xchg(&endpoint->replenish_saved, 0)))
- atomic_add(saved, &endpoint->replenish_backlog);
/* Start replenishing if hardware currently has no buffers */
max_backlog = gsi_channel_tre_max(gsi, endpoint->channel_id);
@@ -1161,11 +1157,7 @@ static void ipa_endpoint_replenish_enable(struct ipa_endpoint *endpoint)
static void ipa_endpoint_replenish_disable(struct ipa_endpoint *endpoint)
{
- u32 backlog;
-
clear_bit(IPA_REPLENISH_ENABLED, endpoint->replenish_flags);
- while ((backlog = atomic_xchg(&endpoint->replenish_backlog, 0)))
- atomic_add(backlog, &endpoint->replenish_saved);
}
static void ipa_endpoint_replenish_work(struct work_struct *work)
@@ -1727,9 +1719,8 @@ static void ipa_endpoint_setup_one(struct ipa_endpoint *endpoint)
*/
clear_bit(IPA_REPLENISH_ENABLED, endpoint->replenish_flags);
clear_bit(IPA_REPLENISH_ACTIVE, endpoint->replenish_flags);
- atomic_set(&endpoint->replenish_saved,
+ atomic_set(&endpoint->replenish_backlog,
gsi_channel_tre_max(gsi, endpoint->channel_id));
- atomic_set(&endpoint->replenish_backlog, 0);
INIT_DELAYED_WORK(&endpoint->replenish_work,
ipa_endpoint_replenish_work);
}
diff --git a/drivers/net/ipa/ipa_endpoint.h b/drivers/net/ipa/ipa_endpoint.h
index 0313cdc607de3..c95816d882a74 100644
--- a/drivers/net/ipa/ipa_endpoint.h
+++ b/drivers/net/ipa/ipa_endpoint.h
@@ -66,7 +66,6 @@ enum ipa_replenish_flag {
* @netdev: Network device pointer, if endpoint uses one
* @replenish_flags: Replenishing state flags
* @replenish_ready: Number of replenish transactions without doorbell
- * @replenish_saved: Replenish requests held while disabled
* @replenish_backlog: Number of buffers needed to fill hardware queue
* @replenish_work: Work item used for repeated replenish failures
*/
@@ -87,7 +86,6 @@ struct ipa_endpoint {
/* Receive buffer replenishing for RX endpoints */
DECLARE_BITMAP(replenish_flags, IPA_REPLENISH_COUNT);
u32 replenish_ready;
- atomic_t replenish_saved;
atomic_t replenish_backlog;
struct delayed_work replenish_work; /* global wq */
};
--
2.32.0