linux-kernel - [PATCH net-next 1/5] net: ipa: always use transaction IDs instead of lists

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220906171942.957704-2-elder@linaro.org>
Date:   Tue,  6 Sep 2022 12:19:38 -0500
From:   Alex Elder <elder@...aro.org>
To:     davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com
Cc:     mka@...omium.org, evgreen@...omium.org, bjorn.andersson@...aro.org,
        quic_cpratapa@...cinc.com, quic_avuyyuru@...cinc.com,
        quic_jponduru@...cinc.com, quic_subashab@...cinc.com,
        elder@...nel.org, netdev@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH net-next 1/5] net: ipa: always use transaction IDs instead of lists

In gsi_channel_trans_complete(), use the completed and pending IDs
to determine whether there are any transactions in completed state.

Similarly, in gsi_channel_trans_cancel_pending(), use the pending
and committed IDs to mark pending transactions cancelled.  Rearrange
the logic a bit there for a simpler result.

This removes the only user of list_last_entry_or_null(), so get rid
of that macro.

Remove the temporary warnings added by the previous commit.

Signed-off-by: Alex Elder <elder@...aro.org>
---
 drivers/net/ipa/gsi_private.h | 14 ---------
 drivers/net/ipa/gsi_trans.c   | 58 ++++++++++-------------------------
 2 files changed, 16 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ipa/gsi_private.h b/drivers/net/ipa/gsi_private.h
index 51bbc7a40dc2d..0b2516fa21b5d 100644
--- a/drivers/net/ipa/gsi_private.h
+++ b/drivers/net/ipa/gsi_private.h
@@ -16,20 +16,6 @@ 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 05ab4d052c68b..a131a4fbb53fc 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -239,22 +239,11 @@ struct gsi_trans *gsi_channel_trans_complete(struct gsi_channel *channel)
 {
 	struct gsi_trans_info *trans_info = &channel->trans_info;
 	u16 trans_id = trans_info->completed_id;
-	struct gsi_trans *trans;
 
-	trans = list_first_entry_or_null(&trans_info->complete,
-					 struct gsi_trans, links);
-
-	if (!trans) {
-		WARN_ON(trans_id != trans_info->pending_id);
+	if (trans_id == trans_info->pending_id)
 		return NULL;
-	}
 
-	if (!WARN_ON(trans_id == trans_info->pending_id)) {
-		trans_id %= channel->tre_count;
-		WARN_ON(trans != &trans_info->trans[trans_id]);
-	}
-
-	return trans;
+	return &trans_info->trans[trans_id %= channel->tre_count];
 }
 
 /* Move a transaction from the allocated list to the committed list */
@@ -705,47 +694,32 @@ void gsi_trans_complete(struct gsi_trans *trans)
 void gsi_channel_trans_cancel_pending(struct gsi_channel *channel)
 {
 	struct gsi_trans_info *trans_info = &channel->trans_info;
-	struct gsi_trans *trans;
-	struct gsi_trans *first;
-	struct gsi_trans *last;
-	bool cancelled;
+	u16 trans_id = trans_info->pending_id;
 
 	/* channel->gsi->mutex is held by caller */
 	spin_lock_bh(&trans_info->spinlock);
 
-	cancelled = !list_empty(&trans_info->pending);
-	list_for_each_entry(trans, &trans_info->pending, links)
-		trans->cancelled = true;
-
 	list_splice_tail_init(&trans_info->pending, &trans_info->complete);
 
-	first = list_first_entry_or_null(&trans_info->complete,
-					 struct gsi_trans, links);
-	last = list_last_entry_or_null(&trans_info->complete,
-				       struct gsi_trans, links);
-
 	spin_unlock_bh(&trans_info->spinlock);
 
+	/* If there are no pending transactions, we're done */
+	if (trans_id == trans_info->committed_id)
+		return;
+
+	/* Mark all pending transactions cancelled */
+	do {
+		struct gsi_trans *trans;
+
+		trans = &trans_info->trans[trans_id % channel->tre_count];
+		trans->cancelled = true;
+	} while (++trans_id != trans_info->committed_id);
+
 	/* All pending transactions are now completed */
-	WARN_ON(cancelled != (trans_info->pending_id !=
-				trans_info->committed_id));
-
 	trans_info->pending_id = trans_info->committed_id;
 
 	/* Schedule NAPI polling to complete the cancelled transactions */
-	if (cancelled) {
-		u16 trans_id;
-
-		napi_schedule(&channel->napi);
-
-		trans_id = trans_info->completed_id;
-		trans = &trans_info->trans[trans_id % channel->tre_count];
-		WARN_ON(trans != first);
-
-		trans_id = trans_info->pending_id - 1;
-		trans = &trans_info->trans[trans_id % channel->tre_count];
-		WARN_ON(trans != last);
-	}
+	napi_schedule(&channel->napi);
 }
 
 /* Issue a command to read a single byte from a channel */
-- 
2.34.1