The first patch in this series makes the name used for variables
representing a TRE ring be consistent everywhere. The second
renames two structure fields to better represent their purpose.
The last four rework a little code that manages some tranaction and
byte transfer statistics maintained mainly for TX endpoints. For
the most part this series is refactoring. The last one also
includes the first step toward no longer assuming an event ring is
dedicated to a single channel.
-Alex
Alex Elder (6):
net: ipa: use "tre_ring" for all TRE ring local variables
net: ipa: rename two transaction fields
net: ipa: introduce gsi_trans_tx_committed()
net: ipa: simplify TX completion statistics
net: ipa: stop counting total RX bytes and transactions
net: ipa: rework gsi_channel_tx_update()
drivers/net/ipa/gsi.c | 77 +++++++++++++++++------------------
drivers/net/ipa/gsi.h | 2 +-
drivers/net/ipa/gsi_private.h | 9 ++++
drivers/net/ipa/gsi_trans.c | 68 +++++++++++++++----------------
drivers/net/ipa/gsi_trans.h | 15 +++----
5 files changed, 87 insertions(+), 84 deletions(-)
--
2.34.1
When a TX request is issued, its channel's accumulated byte and
transaction counts are recorded. This currently does *not* take
into account the transaction being committed.
Later, when the transaction completes, the number of bytes and
transactions that have completed since the transaction was committed
are reported to the network stack. The transaction and its byte
count are accounted for at that time.
Instead, record the transaction and its bytes in the counts recorded
at commit time. This avoids the need to do so when the transaction
completes, and provides a (small) simplification of that code.
Signed-off-by: Alex Elder <[email protected]>
---
drivers/net/ipa/gsi.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 1091ac23567d5..4f8187c543824 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -995,11 +995,11 @@ void gsi_trans_tx_committed(struct gsi_trans *trans)
{
struct gsi_channel *channel = &trans->gsi->channel[trans->channel_id];
- trans->trans_count = channel->trans_count;
- trans->byte_count = channel->byte_count;
-
channel->trans_count++;
channel->byte_count += trans->len;
+
+ trans->trans_count = channel->trans_count;
+ trans->byte_count = channel->byte_count;
}
void gsi_trans_tx_queued(struct gsi_trans *trans)
@@ -1047,13 +1047,11 @@ void gsi_trans_tx_queued(struct gsi_trans *trans)
static void
gsi_channel_tx_update(struct gsi_channel *channel, struct gsi_trans *trans)
{
- u64 byte_count = trans->byte_count + trans->len;
- u64 trans_count = trans->trans_count + 1;
+ u64 trans_count = trans->trans_count - channel->compl_trans_count;
+ u64 byte_count = trans->byte_count - channel->compl_byte_count;
- byte_count -= channel->compl_byte_count;
- channel->compl_byte_count += byte_count;
- trans_count -= channel->compl_trans_count;
channel->compl_trans_count += trans_count;
+ channel->compl_byte_count += byte_count;
ipa_gsi_channel_tx_completed(channel->gsi, gsi_channel_id(channel),
trans_count, byte_count);
--
2.34.1
In gsi_evt_ring_rx_update(), we update each transaction so its len
field reflects the actual number of bytes received. In the process,
the total number of transactions and bytes processed on the channel
are summed, and added to a running total for the channel.
But we don't actually use those running totals for RX endpoints.
They're maintained for TX channels to support CoDel when they are
associated with a "real" network device.
So stop maintaining these totals for RX endpoints, and update the
comment where the fields are defined to make it clear they're only
valid for TX channels.
Signed-off-by: Alex Elder <[email protected]>
---
drivers/net/ipa/gsi.c | 8 --------
drivers/net/ipa/gsi.h | 2 +-
2 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 4f8187c543824..c2cafd9247a70 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1374,8 +1374,6 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index)
struct gsi_event *event_done;
struct gsi_event *event;
struct gsi_trans *trans;
- u32 trans_count = 0;
- u32 byte_count = 0;
u32 event_avail;
u32 old_index;
@@ -1399,8 +1397,6 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index)
event_done = gsi_ring_virt(ring, index);
do {
trans->len = __le16_to_cpu(event->len);
- byte_count += trans->len;
- trans_count++;
/* Move on to the next event and transaction */
if (--event_avail)
@@ -1409,10 +1405,6 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index)
event = gsi_ring_virt(ring, 0);
trans = gsi_trans_pool_next(&trans_info->pool, trans);
} while (event != event_done);
-
- /* We record RX bytes when they are received */
- channel->byte_count += byte_count;
- channel->trans_count += trans_count;
}
/* Initialize a ring, including allocating DMA memory for its entries */
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 89dac7fc8c4cb..bad1a78a96ede 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -117,9 +117,9 @@ struct gsi_channel {
struct gsi_ring tre_ring;
u32 evt_ring_id;
+ /* The following counts are used only for TX endpoints */
u64 byte_count; /* total # bytes transferred */
u64 trans_count; /* total # transactions */
- /* The following counts are used only for TX endpoints */
u64 queued_byte_count; /* last reported queued byte count */
u64 queued_trans_count; /* ...and queued trans count */
u64 compl_byte_count; /* last reported completed byte count */
--
2.34.1
Rename gsi_channel_tx_update() to be gsi_trans_tx_completed(), and
pass it just the transaction pointer, deriving the channel from the
transaction. Update the comments above the function to provide a
more concise description of how statistics for TX endpoints are
maintained and used.
Signed-off-by: Alex Elder <[email protected]>
---
drivers/net/ipa/gsi.c | 54 ++++++++++++++++++++-----------------------
1 file changed, 25 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index c2cafd9247a70..df8af1f00fc8b 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1021,40 +1021,36 @@ void gsi_trans_tx_queued(struct gsi_trans *trans)
}
/**
- * gsi_channel_tx_update() - Report completed TX transfers
- * @channel: Channel that has completed transmitting packets
- * @trans: Last transation known to be complete
- *
- * Compute the number of transactions and bytes that have been transferred
- * over a TX channel since the given transaction was committed. Report this
- * information to the network stack.
- *
- * At the time a transaction is committed, we record its channel's
- * committed transaction and byte counts *in the transaction*.
- * Completions are signaled by the hardware with an interrupt, and
- * we can determine the latest completed transaction at that time.
- *
- * The difference between the byte/transaction count recorded in
- * the transaction and the count last time we recorded a completion
- * tells us exactly how much data has been transferred between
- * completions.
- *
- * Calling this each time we learn of a newly-completed transaction
- * allows us to provide accurate information to the network stack
- * about how much work has been completed by the hardware at a given
- * point in time.
+ * gsi_trans_tx_completed() - Report completed TX transactions
+ * @trans: TX channel transaction that has completed
+ *
+ * Report that a transaction on a TX channel has completed. At the time a
+ * transaction is committed, we record *in the transaction* its channel's
+ * committed transaction and byte counts. Transactions are completed in
+ * order, and the difference between the channel's byte/transaction count
+ * when the transaction was committed and when it completes tells us
+ * exactly how much data has been transferred while the transaction was
+ * pending.
+ *
+ * We report this information to the network stack, which uses it to manage
+ * the rate at which data is sent to hardware.
*/
-static void
-gsi_channel_tx_update(struct gsi_channel *channel, struct gsi_trans *trans)
+static void gsi_trans_tx_completed(struct gsi_trans *trans)
{
- u64 trans_count = trans->trans_count - channel->compl_trans_count;
- u64 byte_count = trans->byte_count - channel->compl_byte_count;
+ u32 channel_id = trans->channel_id;
+ struct gsi *gsi = trans->gsi;
+ struct gsi_channel *channel;
+ u32 trans_count;
+ u32 byte_count;
+
+ channel = &gsi->channel[channel_id];
+ trans_count = trans->trans_count - channel->compl_trans_count;
+ byte_count = trans->byte_count - channel->compl_byte_count;
channel->compl_trans_count += trans_count;
channel->compl_byte_count += byte_count;
- ipa_gsi_channel_tx_completed(channel->gsi, gsi_channel_id(channel),
- trans_count, byte_count);
+ ipa_gsi_channel_tx_completed(gsi, channel_id, trans_count, byte_count);
}
/* Channel control interrupt handler */
@@ -1504,7 +1500,7 @@ static struct gsi_trans *gsi_channel_update(struct gsi_channel *channel)
* up the network stack.
*/
if (channel->toward_ipa)
- gsi_channel_tx_update(channel, trans);
+ gsi_trans_tx_completed(trans);
else
gsi_evt_ring_rx_update(evt_ring, index);
--
2.34.1
All local variables that represent event rings are named "ring".
All but two functions that represent a channel's TRE ring with a
local variable use the name "tre_ring". For consistency, use that
name in the two functions that don't fit the pattern.
Signed-off-by: Alex Elder <[email protected]>
---
drivers/net/ipa/gsi_trans.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index 278e467c5430b..e3f3c736c7409 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -549,7 +549,7 @@ static void gsi_trans_tre_fill(struct gsi_tre *dest_tre, dma_addr_t addr,
static void __gsi_trans_commit(struct gsi_trans *trans, bool ring_db)
{
struct gsi_channel *channel = &trans->gsi->channel[trans->channel_id];
- struct gsi_ring *ring = &channel->tre_ring;
+ struct gsi_ring *tre_ring = &channel->tre_ring;
enum ipa_cmd_opcode opcode = IPA_CMD_NONE;
bool bei = channel->toward_ipa;
struct gsi_tre *dest_tre;
@@ -567,8 +567,8 @@ static void __gsi_trans_commit(struct gsi_trans *trans, bool ring_db)
* transfer request, whose opcode is IPA_CMD_NONE.
*/
cmd_opcode = channel->command ? &trans->cmd_opcode[0] : NULL;
- avail = ring->count - ring->index % ring->count;
- dest_tre = gsi_ring_virt(ring, ring->index);
+ avail = tre_ring->count - tre_ring->index % tre_ring->count;
+ dest_tre = gsi_ring_virt(tre_ring, tre_ring->index);
for_each_sg(trans->sgl, sg, trans->used, i) {
bool last_tre = i == trans->used - 1;
dma_addr_t addr = sg_dma_address(sg);
@@ -576,14 +576,14 @@ static void __gsi_trans_commit(struct gsi_trans *trans, bool ring_db)
byte_count += len;
if (!avail--)
- dest_tre = gsi_ring_virt(ring, 0);
+ dest_tre = gsi_ring_virt(tre_ring, 0);
if (cmd_opcode)
opcode = *cmd_opcode++;
gsi_trans_tre_fill(dest_tre, addr, len, last_tre, bei, opcode);
dest_tre++;
}
- ring->index += trans->used;
+ tre_ring->index += trans->used;
if (channel->toward_ipa) {
/* We record TX bytes when they are sent */
@@ -595,7 +595,7 @@ static void __gsi_trans_commit(struct gsi_trans *trans, bool ring_db)
}
/* Associate the last TRE with the transaction */
- gsi_channel_trans_map(channel, ring->index - 1, trans);
+ gsi_channel_trans_map(channel, tre_ring->index - 1, trans);
gsi_trans_move_pending(trans);
@@ -675,7 +675,7 @@ void gsi_channel_trans_cancel_pending(struct gsi_channel *channel)
int gsi_trans_read_byte(struct gsi *gsi, u32 channel_id, dma_addr_t addr)
{
struct gsi_channel *channel = &gsi->channel[channel_id];
- struct gsi_ring *ring = &channel->tre_ring;
+ struct gsi_ring *tre_ring = &channel->tre_ring;
struct gsi_trans_info *trans_info;
struct gsi_tre *dest_tre;
@@ -687,10 +687,10 @@ int gsi_trans_read_byte(struct gsi *gsi, u32 channel_id, dma_addr_t addr)
/* Now fill the the reserved TRE and tell the hardware */
- dest_tre = gsi_ring_virt(ring, ring->index);
+ dest_tre = gsi_ring_virt(tre_ring, tre_ring->index);
gsi_trans_tre_fill(dest_tre, addr, 1, true, false, IPA_CMD_NONE);
- ring->index++;
+ tre_ring->index++;
gsi_channel_doorbell(channel);
return 0;
--
2.34.1
Hello:
This series was applied to netdev/net-next.git (master)
by David S. Miller <[email protected]>:
On Mon, 13 Jun 2022 12:17:53 -0500 you wrote:
> The first patch in this series makes the name used for variables
> representing a TRE ring be consistent everywhere. The second
> renames two structure fields to better represent their purpose.
>
> The last four rework a little code that manages some tranaction and
> byte transfer statistics maintained mainly for TX endpoints. For
> the most part this series is refactoring. The last one also
> includes the first step toward no longer assuming an event ring is
> dedicated to a single channel.
>
> [...]
Here is the summary with links:
- [net-next,1/6] net: ipa: use "tre_ring" for all TRE ring local variables
https://git.kernel.org/netdev/net-next/c/2295947bdaa6
- [net-next,2/6] net: ipa: rename two transaction fields
https://git.kernel.org/netdev/net-next/c/3eeabea6c895
- [net-next,3/6] net: ipa: introduce gsi_trans_tx_committed()
https://git.kernel.org/netdev/net-next/c/4e0f28e9ee4b
- [net-next,4/6] net: ipa: simplify TX completion statistics
https://git.kernel.org/netdev/net-next/c/65d39497fab6
- [net-next,5/6] net: ipa: stop counting total RX bytes and transactions
https://git.kernel.org/netdev/net-next/c/dbad2fa71914
- [net-next,6/6] net: ipa: rework gsi_channel_tx_update()
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html