[PATCH net-next 0/6] net: ipa: use IDs to track transaction state

[PATCH net-next 0/6] net: ipa: use IDs to track transaction state

[Alex Elder]

This series is the first of three groups of changes that simplify
the way the IPA driver tracks the state of its transactions.

Each GSI channel has a fixed number of transactions allocated at
initialization time.  The number allocated matches the number of
TREs in the transfer ring associated with the channel.  This is
because the transfer ring limits the number of transfers that can
ever be underway, and in the worst case, each transaction represents
a single TRE.

Transactions go through various states during their lifetime.
Currently a set of lists keeps track of which transactions are in
each state.  Initially, all transactions are free.  An allocated
transaction is placed on the allocated list.  Once an allocated
transaction is committed, it is moved from the allocated to the
committed list.  When a committed transaction is sent to hardware
(via a doorbell) it is moved to the pending list.  When hardware
signals that some work has completed, transactions are moved to the
completed list.  Finally, when a completed transaction is polled
it's moved to the polled list before being removed when it becomes
free.

Changing a transaction's state thus normally involves manipulating
two lists, and to prevent corruption a spinlock is held while the
lists are updated.

Transactions move through their states in a well-defined sequence
though, and they do so strictly in order.  So transaction 0 is
always allocated before transaction 1; transaction 0 is always
committed before transaction 1; and so on, through completion,
polling, and becoming free.  Because of this, it's sufficient to
just keep track of which transaction is the first in each state.
The rest of the transactions in a given state can be derived from
the first transaction in an "adjacent" state.  As a result, we can
track the state of all transactions with a set of indexes, and can
update these without the need for a spinlock.

This first group of patches just defines the set of indexes that
will be used for this new way of tracking transaction state.  Two
more groups of patches will follow.  I've broken the 17 patches into
these three groups to facilitate review.

					-Alex

Alex Elder (6):
  net: ipa: use an array for transactions
  net: ipa: track allocated transactions with an ID
  net: ipa: track committed transactions with an ID
  net: ipa: track pending transactions with an ID
  net: ipa: track completed transactions with an ID
  net: ipa: track polled transactions with an ID

 drivers/net/ipa/gsi.h       |  9 +++-
 drivers/net/ipa/gsi_trans.c | 99 ++++++++++++++++++++++++++++++-------
 2 files changed, 89 insertions(+), 19 deletions(-)

-- 
2.34.1

[PATCH net-next 1/6] net: ipa: use an array for transactions

[Alex Elder]

Transactions are always allocated one at a time.  The maximum number
of them we could ever need occurs if each TRE is assigned to a
transaction.  So a channel requires no more transactions than the
number of TREs in its transfer ring.  That number is known to be a
power-of-2 less than 65536.

The transaction pool abstraction is used for other things, but for
transactions we can use a simple array of transaction structures,
and use a free index to indicate which entry in the array is the
next one free for allocation.

By having the number of elements in the array be a power-of-2, we
can use an ever-incrementing 16-bit free index, and use it modulo
the array size.  Distinguish a "trans_id" (whose value can exceed
the number of entries in the transaction array) from a "trans_index"
(which is less than the number of entries).

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.h       |  4 +++-
 drivers/net/ipa/gsi_trans.c | 39 +++++++++++++++++++++----------------
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 23de5f67374cf..4a88aec7e7d92 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -82,7 +82,9 @@ struct gsi_trans_pool {
 
 struct gsi_trans_info {
 	atomic_t tre_avail;		/* TREs available for allocation */
-	struct gsi_trans_pool pool;	/* transaction pool */
+
+	u16 free_id;			/* first free trans in array */
+	struct gsi_trans *trans;	/* transaction array */
 	struct gsi_trans **map;		/* TRE -> transaction map */
 
 	struct gsi_trans_pool sg_pool;	/* scatterlist pool */
diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index 18e7e8c405bea..9775e50d0423f 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -343,20 +343,22 @@ struct gsi_trans *gsi_channel_trans_alloc(struct gsi *gsi, u32 channel_id,
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 	struct gsi_trans_info *trans_info;
 	struct gsi_trans *trans;
+	u16 trans_index;
 
 	if (WARN_ON(tre_count > channel->trans_tre_max))
 		return NULL;
 
 	trans_info = &channel->trans_info;
 
-	/* We reserve the TREs now, but consume them at commit time.
-	 * If there aren't enough available, we're done.
-	 */
+	/* If we can't reserve the TREs for the transaction, we're done */
 	if (!gsi_trans_tre_reserve(trans_info, tre_count))
 		return NULL;
 
-	/* Allocate and initialize non-zero fields in the transaction */
-	trans = gsi_trans_pool_alloc(&trans_info->pool, 1);
+	trans_index = trans_info->free_id % channel->tre_count;
+	trans = &trans_info->trans[trans_index];
+	memset(trans, 0, sizeof(*trans));
+
+	/* Initialize non-zero fields in the transaction */
 	trans->gsi = gsi;
 	trans->channel_id = channel_id;
 	trans->rsvd_count = tre_count;
@@ -367,15 +369,17 @@ struct gsi_trans *gsi_channel_trans_alloc(struct gsi *gsi, u32 channel_id,
 	sg_init_marker(trans->sgl, tre_count);
 
 	trans->direction = direction;
-
-	spin_lock_bh(&trans_info->spinlock);
-
-	list_add_tail(&trans->links, &trans_info->alloc);
-
-	spin_unlock_bh(&trans_info->spinlock);
-
 	refcount_set(&trans->refcount, 1);
 
+	/* This free transaction will now be allocated */
+	trans_info->free_id++;
+
+	spin_lock_bh(&trans_info->spinlock);
+
+	list_add_tail(&trans->links, &trans_info->alloc);
+
+	spin_unlock_bh(&trans_info->spinlock);
+
 	return trans;
 }
 
@@ -736,10 +740,11 @@ int gsi_channel_trans_init(struct gsi *gsi, u32 channel_id)
 	 * modulo that number to determine the next one that's free.
 	 * Transactions are allocated one at a time.
 	 */
-	ret = gsi_trans_pool_init(&trans_info->pool, sizeof(struct gsi_trans),
-				  tre_max, 1);
-	if (ret)
+	trans_info->trans = kcalloc(tre_count, sizeof(*trans_info->trans),
+				    GFP_KERNEL);
+	if (!trans_info->trans)
 		return -ENOMEM;
+	trans_info->free_id = 0;	/* modulo channel->tre_count */
 
 	/* A completion event contains a pointer to the TRE that caused
 	 * the event (which will be the last one used by the transaction).
@@ -777,7 +782,7 @@ int gsi_channel_trans_init(struct gsi *gsi, u32 channel_id)
 err_map_free:
 	kfree(trans_info->map);
 err_trans_free:
-	gsi_trans_pool_exit(&trans_info->pool);
+	kfree(trans_info->trans);
 
 	dev_err(gsi->dev, "error %d initializing channel %u transactions\n",
 		ret, channel_id);
@@ -791,6 +796,6 @@ void gsi_channel_trans_exit(struct gsi_channel *channel)
 	struct gsi_trans_info *trans_info = &channel->trans_info;
 
 	gsi_trans_pool_exit(&trans_info->sg_pool);
-	gsi_trans_pool_exit(&trans_info->pool);
+	kfree(trans_info->trans);
 	kfree(trans_info->map);
 }
-- 
2.34.1

[PATCH net-next 2/6] net: ipa: track allocated transactions with an ID

[Alex Elder]

Transactions for a channel are now managed in an array, with a free
transaction ID indicating which is the next one free.

Add another transaction ID field to track the first element in the
array that has been allocated.  Advance it when a transaction is
committed (because that is when that transaction leaves allocated
state).

Temporarily add warnings that verify that the first allocated
transaction tracked by the ID matches the first element on the
allocated list, both when allocating and committing a transaction.

Signed-off-by: Alex Elder <elder@linaro.org>
---

NOTE: I find these temporary WARN_ON() calls helpful in proving
      the new index refers to the same transaction as the first
      element of an "old" list.  I'll gladly remove these if
      requested.  (This comment applies throughout this series.)

 drivers/net/ipa/gsi.h       |  1 +
 drivers/net/ipa/gsi_trans.c | 28 ++++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 4a88aec7e7d92..6bbbda6f27eae 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -84,6 +84,7 @@ struct gsi_trans_info {
 	atomic_t tre_avail;		/* TREs available for allocation */
 
 	u16 free_id;			/* first free trans in array */
+	u16 allocated_id;		/* first allocated transaction */
 	struct gsi_trans *trans;	/* transaction array */
 	struct gsi_trans **map;		/* TRE -> transaction map */
 
diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index 9775e50d0423f..d84400e13487f 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -246,12 +246,26 @@ static void gsi_trans_move_committed(struct gsi_trans *trans)
 {
 	struct gsi_channel *channel = &trans->gsi->channel[trans->channel_id];
 	struct gsi_trans_info *trans_info = &channel->trans_info;
+	u16 trans_index;
 
 	spin_lock_bh(&trans_info->spinlock);
 
 	list_move_tail(&trans->links, &trans_info->committed);
 
+	trans = list_first_entry_or_null(&trans_info->alloc,
+					 struct gsi_trans, links);
+
 	spin_unlock_bh(&trans_info->spinlock);
+
+	/* This allocated transaction is now committed */
+	trans_info->allocated_id++;
+
+	if (trans) {
+		trans_index = trans_info->allocated_id % channel->tre_count;
+		WARN_ON(trans != &trans_info->trans[trans_index]);
+	} else {
+		WARN_ON(trans_info->allocated_id != trans_info->free_id);
+	}
 }
 
 /* Move transactions from the committed list to the pending list */
@@ -378,8 +392,14 @@ struct gsi_trans *gsi_channel_trans_alloc(struct gsi *gsi, u32 channel_id,
 
 	list_add_tail(&trans->links, &trans_info->alloc);
 
+	trans = list_first_entry(&trans_info->alloc, struct gsi_trans, links);
+
 	spin_unlock_bh(&trans_info->spinlock);
 
+	WARN_ON(trans_info->allocated_id == trans_info->free_id);
+	trans_index = trans_info->allocated_id % channel->tre_count;
+	WARN_ON(trans != &trans_info->trans[trans_index]);
+
 	return trans;
 }
 
@@ -408,7 +428,10 @@ void gsi_trans_free(struct gsi_trans *trans)
 	if (!last)
 		return;
 
-	if (trans->used_count)
+	/* Unused transactions are allocated but never committed */
+	if (!trans->used_count)
+		trans_info->allocated_id++;
+	else
 		ipa_gsi_trans_release(trans);
 
 	/* Releasing the reserved TREs implicitly frees the sgl[] and
@@ -744,7 +767,8 @@ int gsi_channel_trans_init(struct gsi *gsi, u32 channel_id)
 				    GFP_KERNEL);
 	if (!trans_info->trans)
 		return -ENOMEM;
-	trans_info->free_id = 0;	/* modulo channel->tre_count */
+	trans_info->free_id = 0;	/* all modulo channel->tre_count */
+	trans_info->allocated_id = 0;
 
 	/* A completion event contains a pointer to the TRE that caused
 	 * the event (which will be the last one used by the transaction).
-- 
2.34.1

[PATCH net-next 3/6] net: ipa: track committed transactions with an ID

[Alex Elder]

Add a transaction ID field to track the first element in a channel's
transaction array that has been committed, but not yet passed to the
hardware.  Advance the ID when the hardware is notified via doorbell
that TREs from a transaction are ready for consumption.

Temporarily add warnings that verify that the first committed
transaction tracked by the ID matches the first element on the
committed list, both when committing and pending (at doorbell).

Remove the temporary warnings added by the previous commit.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.h       |  1 +
 drivers/net/ipa/gsi_trans.c | 45 +++++++++++++++++++++++--------------
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 6bbbda6f27eae..cc46a9119fc5b 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -85,6 +85,7 @@ struct gsi_trans_info {
 
 	u16 free_id;			/* first free trans in array */
 	u16 allocated_id;		/* first allocated transaction */
+	u16 committed_id;		/* first committed transaction */
 	struct gsi_trans *trans;	/* transaction array */
 	struct gsi_trans **map;		/* TRE -> transaction map */
 
diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index d84400e13487f..72da795908fee 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -252,20 +252,17 @@ static void gsi_trans_move_committed(struct gsi_trans *trans)
 
 	list_move_tail(&trans->links, &trans_info->committed);
 
-	trans = list_first_entry_or_null(&trans_info->alloc,
-					 struct gsi_trans, links);
+	trans = list_first_entry(&trans_info->committed,
+				 struct gsi_trans, links);
 
 	spin_unlock_bh(&trans_info->spinlock);
 
 	/* This allocated transaction is now committed */
 	trans_info->allocated_id++;
 
-	if (trans) {
-		trans_index = trans_info->allocated_id % channel->tre_count;
-		WARN_ON(trans != &trans_info->trans[trans_index]);
-	} else {
-		WARN_ON(trans_info->allocated_id != trans_info->free_id);
-	}
+	WARN_ON(trans_info->committed_id == trans_info->allocated_id);
+	trans_index = trans_info->committed_id % channel->tre_count;
+	WARN_ON(trans != &trans_info->trans[trans_index]);
 }
 
 /* Move transactions from the committed list to the pending list */
@@ -273,7 +270,9 @@ static void gsi_trans_move_pending(struct gsi_trans *trans)
 {
 	struct gsi_channel *channel = &trans->gsi->channel[trans->channel_id];
 	struct gsi_trans_info *trans_info = &channel->trans_info;
+	u16 trans_index = trans - &trans_info->trans[0];
 	struct list_head list;
+	u16 delta;
 
 	spin_lock_bh(&trans_info->spinlock);
 
@@ -281,7 +280,22 @@ static void gsi_trans_move_pending(struct gsi_trans *trans)
 	list_cut_position(&list, &trans_info->committed, &trans->links);
 	list_splice_tail(&list, &trans_info->pending);
 
+	trans = list_first_entry_or_null(&trans_info->committed,
+					 struct gsi_trans, links);
+
 	spin_unlock_bh(&trans_info->spinlock);
+
+	/* These committed transactions are now pending */
+	delta = trans_index - trans_info->committed_id + 1;
+	trans_info->committed_id += delta % channel->tre_count;
+
+	if (trans) {
+		trans_index = trans_info->committed_id % channel->tre_count;
+		WARN_ON(trans != &trans_info->trans[trans_index]);
+	} else {
+		WARN_ON(trans_info->committed_id !=
+			trans_info->allocated_id);
+	}
 }
 
 /* Move a transaction and all of its predecessors from the pending list
@@ -392,14 +406,8 @@ struct gsi_trans *gsi_channel_trans_alloc(struct gsi *gsi, u32 channel_id,
 
 	list_add_tail(&trans->links, &trans_info->alloc);
 
-	trans = list_first_entry(&trans_info->alloc, struct gsi_trans, links);
-
 	spin_unlock_bh(&trans_info->spinlock);
 
-	WARN_ON(trans_info->allocated_id == trans_info->free_id);
-	trans_index = trans_info->allocated_id % channel->tre_count;
-	WARN_ON(trans != &trans_info->trans[trans_index]);
-
 	return trans;
 }
 
@@ -428,11 +436,13 @@ void gsi_trans_free(struct gsi_trans *trans)
 	if (!last)
 		return;
 
-	/* Unused transactions are allocated but never committed */
-	if (!trans->used_count)
+	/* Unused transactions are allocated but never committed or pending */
+	if (!trans->used_count) {
 		trans_info->allocated_id++;
-	else
+		trans_info->committed_id++;
+	} else {
 		ipa_gsi_trans_release(trans);
+	}
 
 	/* Releasing the reserved TREs implicitly frees the sgl[] and
 	 * (if present) info[] arrays, plus the transaction itself.
@@ -769,6 +779,7 @@ int gsi_channel_trans_init(struct gsi *gsi, u32 channel_id)
 		return -ENOMEM;
 	trans_info->free_id = 0;	/* all modulo channel->tre_count */
 	trans_info->allocated_id = 0;
+	trans_info->committed_id = 0;
 
 	/* A completion event contains a pointer to the TRE that caused
 	 * the event (which will be the last one used by the transaction).
-- 
2.34.1

[PATCH net-next 4/6] net: ipa: track pending transactions with an ID

[Alex Elder]

Add a transaction ID field to track the first element in the
transaction array that is pending (sent to hardware) but not yet
complete.  Advance the ID when a completion event for a channel
indicates that transactions have completed.

Temporarily add warnings that verify that the first pending
transaction tracked by the ID matches the first element on the
pending list, both when pending and completing, as well as when
resetting the channel.

Remove the temporary warnings added by the previous commit.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.h       |  1 +
 drivers/net/ipa/gsi_trans.c | 46 ++++++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index cc46a9119fc5b..f23e7e562585e 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -86,6 +86,7 @@ struct gsi_trans_info {
 	u16 free_id;			/* first free trans in array */
 	u16 allocated_id;		/* first allocated transaction */
 	u16 committed_id;		/* first committed transaction */
+	u16 pending_id;			/* first pending transaction */
 	struct gsi_trans *trans;	/* transaction array */
 	struct gsi_trans **map;		/* TRE -> transaction map */
 
diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index 72da795908fee..5e3b4f673d9fb 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -246,23 +246,15 @@ static void gsi_trans_move_committed(struct gsi_trans *trans)
 {
 	struct gsi_channel *channel = &trans->gsi->channel[trans->channel_id];
 	struct gsi_trans_info *trans_info = &channel->trans_info;
-	u16 trans_index;
 
 	spin_lock_bh(&trans_info->spinlock);
 
 	list_move_tail(&trans->links, &trans_info->committed);
 
-	trans = list_first_entry(&trans_info->committed,
-				 struct gsi_trans, links);
-
 	spin_unlock_bh(&trans_info->spinlock);
 
 	/* This allocated transaction is now committed */
 	trans_info->allocated_id++;
-
-	WARN_ON(trans_info->committed_id == trans_info->allocated_id);
-	trans_index = trans_info->committed_id % channel->tre_count;
-	WARN_ON(trans != &trans_info->trans[trans_index]);
 }
 
 /* Move transactions from the committed list to the pending list */
@@ -280,8 +272,8 @@ static void gsi_trans_move_pending(struct gsi_trans *trans)
 	list_cut_position(&list, &trans_info->committed, &trans->links);
 	list_splice_tail(&list, &trans_info->pending);
 
-	trans = list_first_entry_or_null(&trans_info->committed,
-					 struct gsi_trans, links);
+	trans = list_first_entry(&trans_info->pending,
+				 struct gsi_trans, links);
 
 	spin_unlock_bh(&trans_info->spinlock);
 
@@ -289,13 +281,9 @@ static void gsi_trans_move_pending(struct gsi_trans *trans)
 	delta = trans_index - trans_info->committed_id + 1;
 	trans_info->committed_id += delta % channel->tre_count;
 
-	if (trans) {
-		trans_index = trans_info->committed_id % channel->tre_count;
-		WARN_ON(trans != &trans_info->trans[trans_index]);
-	} else {
-		WARN_ON(trans_info->committed_id !=
-			trans_info->allocated_id);
-	}
+	WARN_ON(trans_info->pending_id == trans_info->committed_id);
+	trans_index = trans_info->pending_id % channel->tre_count;
+	WARN_ON(trans != &trans_info->trans[trans_index]);
 }
 
 /* Move a transaction and all of its predecessors from the pending list
@@ -305,7 +293,9 @@ void gsi_trans_move_complete(struct gsi_trans *trans)
 {
 	struct gsi_channel *channel = &trans->gsi->channel[trans->channel_id];
 	struct gsi_trans_info *trans_info = &channel->trans_info;
+	u16 trans_index = trans - trans_info->trans;
 	struct list_head list;
+	u16 delta;
 
 	spin_lock_bh(&trans_info->spinlock);
 
@@ -313,7 +303,23 @@ void gsi_trans_move_complete(struct gsi_trans *trans)
 	list_cut_position(&list, &trans_info->pending, &trans->links);
 	list_splice_tail(&list, &trans_info->complete);
 
+	trans = list_first_entry_or_null(&trans_info->pending,
+					 struct gsi_trans, links);
+
 	spin_unlock_bh(&trans_info->spinlock);
+
+	/* These pending transactions are now completed */
+	delta = trans_index - trans_info->pending_id + 1;
+	delta %= channel->tre_count;
+	trans_info->pending_id += delta;
+
+	if (trans) {
+		trans_index = trans_info->pending_id % channel->tre_count;
+		WARN_ON(trans != &trans_info->trans[trans_index]);
+	} else {
+		WARN_ON(trans_info->pending_id !=
+			trans_info->committed_id);
+	}
 }
 
 /* Move a transaction from the completed list to the polled list */
@@ -436,10 +442,13 @@ void gsi_trans_free(struct gsi_trans *trans)
 	if (!last)
 		return;
 
-	/* Unused transactions are allocated but never committed or pending */
+	/* Unused transactions are allocated but never committed, pending,
+	 * or completed.
+	 */
 	if (!trans->used_count) {
 		trans_info->allocated_id++;
 		trans_info->committed_id++;
+		trans_info->pending_id++;
 	} else {
 		ipa_gsi_trans_release(trans);
 	}
@@ -780,6 +789,7 @@ int gsi_channel_trans_init(struct gsi *gsi, u32 channel_id)
 	trans_info->free_id = 0;	/* all modulo channel->tre_count */
 	trans_info->allocated_id = 0;
 	trans_info->committed_id = 0;
+	trans_info->pending_id = 0;
 
 	/* A completion event contains a pointer to the TRE that caused
 	 * the event (which will be the last one used by the transaction).
-- 
2.34.1

[PATCH net-next 5/6] net: ipa: track completed transactions with an ID

[Alex Elder]

Add a transaction ID field to track the first element in the
transaction array that has completed but has not yet been polled.

Advance the ID when we are processing a transaction in the NAPI
polling loop (where completed transactions become polled).

Temporarily add warnings that verify that the first completed
transaction tracked by the ID matches the first element on the
completed list, both when pending and completing.

Remove the temporary warnings added by the previous commit.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.h       |  1 +
 drivers/net/ipa/gsi_trans.c | 40 +++++++++++++++++++++----------------
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index f23e7e562585e..987f9f5f35d36 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -87,6 +87,7 @@ struct gsi_trans_info {
 	u16 allocated_id;		/* first allocated transaction */
 	u16 committed_id;		/* first committed transaction */
 	u16 pending_id;			/* first pending transaction */
+	u16 completed_id;		/* first completed transaction */
 	struct gsi_trans *trans;	/* transaction array */
 	struct gsi_trans **map;		/* TRE -> transaction map */
 
diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index 5e3b4f673d9fb..40852b1dd5b98 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -272,18 +272,11 @@ static void gsi_trans_move_pending(struct gsi_trans *trans)
 	list_cut_position(&list, &trans_info->committed, &trans->links);
 	list_splice_tail(&list, &trans_info->pending);
 
-	trans = list_first_entry(&trans_info->pending,
-				 struct gsi_trans, links);
-
 	spin_unlock_bh(&trans_info->spinlock);
 
 	/* These committed transactions are now pending */
 	delta = trans_index - trans_info->committed_id + 1;
 	trans_info->committed_id += delta % channel->tre_count;
-
-	WARN_ON(trans_info->pending_id == trans_info->committed_id);
-	trans_index = trans_info->pending_id % channel->tre_count;
-	WARN_ON(trans != &trans_info->trans[trans_index]);
 }
 
 /* Move a transaction and all of its predecessors from the pending list
@@ -303,8 +296,8 @@ void gsi_trans_move_complete(struct gsi_trans *trans)
 	list_cut_position(&list, &trans_info->pending, &trans->links);
 	list_splice_tail(&list, &trans_info->complete);
 
-	trans = list_first_entry_or_null(&trans_info->pending,
-					 struct gsi_trans, links);
+	trans = list_first_entry(&trans_info->complete,
+				 struct gsi_trans, links);
 
 	spin_unlock_bh(&trans_info->spinlock);
 
@@ -313,13 +306,9 @@ void gsi_trans_move_complete(struct gsi_trans *trans)
 	delta %= channel->tre_count;
 	trans_info->pending_id += delta;
 
-	if (trans) {
-		trans_index = trans_info->pending_id % channel->tre_count;
-		WARN_ON(trans != &trans_info->trans[trans_index]);
-	} else {
-		WARN_ON(trans_info->pending_id !=
-			trans_info->committed_id);
-	}
+	WARN_ON(trans_info->completed_id == trans_info->pending_id);
+	trans_index = trans_info->completed_id % channel->tre_count;
+	WARN_ON(trans != &trans_info->trans[trans_index]);
 }
 
 /* Move a transaction from the completed list to the polled list */
@@ -327,12 +316,27 @@ void gsi_trans_move_polled(struct gsi_trans *trans)
 {
 	struct gsi_channel *channel = &trans->gsi->channel[trans->channel_id];
 	struct gsi_trans_info *trans_info = &channel->trans_info;
+	u16 trans_index;
 
 	spin_lock_bh(&trans_info->spinlock);
 
 	list_move_tail(&trans->links, &trans_info->polled);
 
+	trans = list_first_entry_or_null(&trans_info->complete,
+					 struct gsi_trans, links);
+
 	spin_unlock_bh(&trans_info->spinlock);
+
+	/* This completed transaction is now polled */
+	trans_info->completed_id++;
+
+	if (trans) {
+		trans_index = trans_info->completed_id % channel->tre_count;
+		WARN_ON(trans != &trans_info->trans[trans_index]);
+	} else {
+		WARN_ON(trans_info->completed_id !=
+			trans_info->pending_id);
+	}
 }
 
 /* Reserve some number of TREs on a channel.  Returns true if successful */
@@ -443,12 +447,13 @@ void gsi_trans_free(struct gsi_trans *trans)
 		return;
 
 	/* Unused transactions are allocated but never committed, pending,
-	 * or completed.
+	 * completed, or polled.
 	 */
 	if (!trans->used_count) {
 		trans_info->allocated_id++;
 		trans_info->committed_id++;
 		trans_info->pending_id++;
+		trans_info->completed_id++;
 	} else {
 		ipa_gsi_trans_release(trans);
 	}
@@ -790,6 +795,7 @@ int gsi_channel_trans_init(struct gsi *gsi, u32 channel_id)
 	trans_info->allocated_id = 0;
 	trans_info->committed_id = 0;
 	trans_info->pending_id = 0;
+	trans_info->completed_id = 0;
 
 	/* A completion event contains a pointer to the TRE that caused
 	 * the event (which will be the last one used by the transaction).
-- 
2.34.1

[PATCH net-next 6/6] net: ipa: track polled transactions with an ID

[Alex Elder]

Add a transaction ID to track the first element in the transaction
array that has been polled.  Advance the ID when we are releasing a
transaction.

Temporarily add warnings that verify that the first polled
transaction tracked by the ID matches the first element on the
polled list, both when polling and freeing.

Remove the temporary warnings added by the previous commit.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.h       |  1 +
 drivers/net/ipa/gsi_trans.c | 39 ++++++++++++++++++++++---------------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 987f9f5f35d36..13468704c4000 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -88,6 +88,7 @@ struct gsi_trans_info {
 	u16 committed_id;		/* first committed transaction */
 	u16 pending_id;			/* first pending transaction */
 	u16 completed_id;		/* first completed transaction */
+	u16 polled_id;			/* first polled transaction */
 	struct gsi_trans *trans;	/* transaction array */
 	struct gsi_trans **map;		/* TRE -> transaction map */
 
diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index 40852b1dd5b98..4eef1480c2005 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -296,19 +296,12 @@ void gsi_trans_move_complete(struct gsi_trans *trans)
 	list_cut_position(&list, &trans_info->pending, &trans->links);
 	list_splice_tail(&list, &trans_info->complete);
 
-	trans = list_first_entry(&trans_info->complete,
-				 struct gsi_trans, links);
-
 	spin_unlock_bh(&trans_info->spinlock);
 
 	/* These pending transactions are now completed */
 	delta = trans_index - trans_info->pending_id + 1;
 	delta %= channel->tre_count;
 	trans_info->pending_id += delta;
-
-	WARN_ON(trans_info->completed_id == trans_info->pending_id);
-	trans_index = trans_info->completed_id % channel->tre_count;
-	WARN_ON(trans != &trans_info->trans[trans_index]);
 }
 
 /* Move a transaction from the completed list to the polled list */
@@ -322,21 +315,17 @@ void gsi_trans_move_polled(struct gsi_trans *trans)
 
 	list_move_tail(&trans->links, &trans_info->polled);
 
-	trans = list_first_entry_or_null(&trans_info->complete,
-					 struct gsi_trans, links);
+	trans = list_first_entry(&trans_info->polled,
+				 struct gsi_trans, links);
 
 	spin_unlock_bh(&trans_info->spinlock);
 
 	/* This completed transaction is now polled */
 	trans_info->completed_id++;
 
-	if (trans) {
-		trans_index = trans_info->completed_id % channel->tre_count;
-		WARN_ON(trans != &trans_info->trans[trans_index]);
-	} else {
-		WARN_ON(trans_info->completed_id !=
-			trans_info->pending_id);
-	}
+	WARN_ON(trans_info->polled_id == trans_info->completed_id);
+	trans_index = trans_info->polled_id % channel->tre_count;
+	WARN_ON(trans != &trans_info->trans[trans_index]);
 }
 
 /* Reserve some number of TREs on a channel.  Returns true if successful */
@@ -424,8 +413,11 @@ struct gsi_trans *gsi_channel_trans_alloc(struct gsi *gsi, u32 channel_id,
 /* Free a previously-allocated transaction */
 void gsi_trans_free(struct gsi_trans *trans)
 {
+	struct gsi_channel *channel = &trans->gsi->channel[trans->channel_id];
 	refcount_t *refcount = &trans->refcount;
 	struct gsi_trans_info *trans_info;
+	struct gsi_trans *polled;
+	u16 trans_index;
 	bool last;
 
 	/* We must hold the lock to release the last reference */
@@ -441,6 +433,9 @@ void gsi_trans_free(struct gsi_trans *trans)
 	if (last)
 		list_del(&trans->links);
 
+	polled = list_first_entry_or_null(&trans_info->polled,
+					  struct gsi_trans, links);
+
 	spin_unlock_bh(&trans_info->spinlock);
 
 	if (!last)
@@ -458,6 +453,17 @@ void gsi_trans_free(struct gsi_trans *trans)
 		ipa_gsi_trans_release(trans);
 	}
 
+	/* This transaction is now free */
+	trans_info->polled_id++;
+
+	if (polled) {
+		trans_index = trans_info->polled_id % channel->tre_count;
+		WARN_ON(polled != &trans_info->trans[trans_index]);
+	} else {
+		WARN_ON(trans_info->polled_id !=
+			trans_info->completed_id);
+	}
+
 	/* Releasing the reserved TREs implicitly frees the sgl[] and
 	 * (if present) info[] arrays, plus the transaction itself.
 	 */
@@ -796,6 +802,7 @@ int gsi_channel_trans_init(struct gsi *gsi, u32 channel_id)
 	trans_info->committed_id = 0;
 	trans_info->pending_id = 0;
 	trans_info->completed_id = 0;
+	trans_info->polled_id = 0;
 
 	/* A completion event contains a pointer to the TRE that caused
 	 * the event (which will be the last one used by the transaction).
-- 
2.34.1

Re: [PATCH net-next 0/6] net: ipa: use IDs to track transaction state

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Wed, 31 Aug 2022 17:40:11 -0500 you wrote:
> This series is the first of three groups of changes that simplify
> the way the IPA driver tracks the state of its transactions.
> 
> Each GSI channel has a fixed number of transactions allocated at
> initialization time.  The number allocated matches the number of
> TREs in the transfer ring associated with the channel.  This is
> because the transfer ring limits the number of transfers that can
> ever be underway, and in the worst case, each transaction represents
> a single TRE.
> 
> [...]

Here is the summary with links:
  - [net-next,1/6] net: ipa: use an array for transactions
    (no matching commit)
  - [net-next,2/6] net: ipa: track allocated transactions with an ID
    https://git.kernel.org/netdev/net-next/c/41e2a2c054b8
  - [net-next,3/6] net: ipa: track committed transactions with an ID
    https://git.kernel.org/netdev/net-next/c/fc95d958e27d
  - [net-next,4/6] net: ipa: track pending transactions with an ID
    https://git.kernel.org/netdev/net-next/c/eeff7c14e08c
  - [net-next,5/6] net: ipa: track completed transactions with an ID
    https://git.kernel.org/netdev/net-next/c/949cd0b5c296
  - [net-next,6/6] net: ipa: track polled transactions with an ID
    https://git.kernel.org/netdev/net-next/c/fd3bd0398a0d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html