diff mbox series

[net-next,1/6] net: ipa: rework last transaction determination

Message ID 20220902210218.745873-2-elder@linaro.org (mailing list archive)
State Accepted
Commit b2abe33d23cfaea6cd3f8335a1ee08c480512c6f
Delegated to: Netdev Maintainers
Headers show
Series net: ipa: start using transaction IDs | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 127 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Alex Elder Sept. 2, 2022, 9:02 p.m. UTC
When quiescing a channel, we find the "last" transaction, which is
the latest one to have been allocated.  (New transaction allocation
will have been prevented by the time this is called.)

Currently we do this by looking for the first non-empty transaction
list in each state, then return the last entry from that last.
Instead, determine the last entry in each list (if any) and return
that entry if found.

Temporarily (locally) introduce list_last_entry_or_null() as a
helper for this, mirroring list_first_entry_or_null().  This macro
definition will be removed by an upcoming patch.

Remove the temporary warnings added by the previous commit.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c         | 28 ++++++++++++++--------------
 drivers/net/ipa/gsi_private.h | 14 ++++++++++++++
 drivers/net/ipa/gsi_trans.c   | 22 ----------------------
 3 files changed, 28 insertions(+), 36 deletions(-)
diff mbox series

Patch

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 9e307eebd33f9..0ea98fa5dee56 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -710,7 +710,6 @@  static void gsi_evt_ring_program(struct gsi *gsi, u32 evt_ring_id)
 static struct gsi_trans *gsi_channel_trans_last(struct gsi_channel *channel)
 {
 	struct gsi_trans_info *trans_info = &channel->trans_info;
-	const struct list_head *list;
 	struct gsi_trans *trans;
 
 	spin_lock_bh(&trans_info->spinlock);
@@ -719,29 +718,30 @@  static struct gsi_trans *gsi_channel_trans_last(struct gsi_channel *channel)
 	 * before we disabled transmits, so check for that.
 	 */
 	if (channel->toward_ipa) {
-		list = &trans_info->alloc;
-		if (!list_empty(list))
+		trans = list_last_entry_or_null(&trans_info->alloc,
+						struct gsi_trans, links);
+		if (trans)
 			goto done;
-		list = &trans_info->committed;
-		if (!list_empty(list))
+		trans = list_last_entry_or_null(&trans_info->committed,
+						struct gsi_trans, links);
+		if (trans)
 			goto done;
-		list = &trans_info->pending;
-		if (!list_empty(list))
+		trans = list_last_entry_or_null(&trans_info->pending,
+						struct gsi_trans, links);
+		if (trans)
 			goto done;
 	}
 
 	/* Otherwise (TX or RX) we want to wait for anything that
 	 * has completed, or has been polled but not released yet.
 	 */
-	list = &trans_info->complete;
-	if (!list_empty(list))
+	trans = list_last_entry_or_null(&trans_info->complete,
+					struct gsi_trans, links);
+	if (trans)
 		goto done;
-	list = &trans_info->polled;
-	if (list_empty(list))
-		list = NULL;
+	trans = list_last_entry_or_null(&trans_info->polled,
+					struct gsi_trans, links);
 done:
-	trans = list ? list_last_entry(list, struct gsi_trans, links) : NULL;
-
 	/* Caller will wait for this, so take a reference */
 	if (trans)
 		refcount_inc(&trans->refcount);
diff --git a/drivers/net/ipa/gsi_private.h b/drivers/net/ipa/gsi_private.h
index 0b2516fa21b5d..51bbc7a40dc2d 100644
--- a/drivers/net/ipa/gsi_private.h
+++ b/drivers/net/ipa/gsi_private.h
@@ -16,6 +16,20 @@  struct gsi_channel;
 
 #define GSI_RING_ELEMENT_SIZE	16	/* bytes; must be a power of 2 */
 
+/**
+ * list_last_entry_or_null - get the last element from a list
+ * @ptr:	the list head to take the element from.
+ * @type:	the type of the struct this is embedded in.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Note that if the list is empty, it returns NULL.
+ */
+#define list_last_entry_or_null(ptr, type, member) ({ \
+	struct list_head *head__ = (ptr); \
+	struct list_head *pos__ = READ_ONCE(head__->prev); \
+	pos__ != head__ ? list_entry(pos__, type, member) : NULL; \
+})
+
 /**
  * gsi_trans_move_complete() - Mark a GSI transaction completed
  * @trans:	Transaction to commit
diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index 4eef1480c2005..b4a6f2b563566 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -309,23 +309,15 @@  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(&trans_info->polled,
-				 struct gsi_trans, links);
-
 	spin_unlock_bh(&trans_info->spinlock);
 
 	/* This completed transaction is now polled */
 	trans_info->completed_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 */
@@ -413,11 +405,8 @@  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 */
@@ -433,9 +422,6 @@  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)
@@ -456,14 +442,6 @@  void gsi_trans_free(struct gsi_trans *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.
 	 */