Re: [PATCH net-next 02/15] net: dsa: sja1105: Remove usage of iterator for list_add() after loop

On Sun, Apr 10, 2022 at 10:30:31PM +0200, Jakob Koschel wrote:

quoted

On 10. Apr 2022, at 22:02, Vladimir Oltean [off-list ref] wrote:

On Sun, Apr 10, 2022 at 08:24:37PM +0200, Jakob Koschel wrote:

quoted

Btw, I just realized that the if (!pos) is not necessary. This should simply do it:

diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index b7e95d60a6e4..2d59e75a9e3d 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c

@@ -28,6 +28,7 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,

		list_add(&e->list, &gating_cfg->entries);
	} else {
+		struct list_head *pos = &gating_cfg->entries;
		struct sja1105_gate_entry *p;

		list_for_each_entry(p, &gating_cfg->entries, list) {
			if (p->interval == e->interval) {

@@ -37,10 +38,12 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,

				goto err;
			}

-			if (e->interval < p->interval)
+			if (e->interval < p->interval) {
+				pos = &p->list;
				break;
+			}
		}
-		list_add(&e->list, p->list.prev);
+		list_add(&e->list, pos->prev);
	}

	gating_cfg->num_entries++;
-- 
2.25.1

I think we can give this a turn that is actually beneficial for the driver.
I've prepared and tested 3 patches on this function, see below.
Concrete improvements:
- that thing with list_for_each_entry() and list_for_each()
- no more special-casing of an empty list
- simplifying the error path
- that thing with list_add_tail()

What do you think about the changes below?

Thanks for all the good cooperation and help. The changes look great.
I'll include them in v2 unless you want to do that separately, then I'll
just remove them from the patch series.

I think it's less of a synchronization hassle if you send them along
with your list of others. Good luck.