Messages in this thread |  | | Date | Sun, 10 Apr 2022 23:34:57 +0300 | From | Vladimir Oltean <> | Subject | 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: > > On 10. Apr 2022, at 22:02, Vladimir Oltean <olteanv@gmail.com> wrote: > > > > On Sun, Apr 10, 2022 at 08:24:37PM +0200, Jakob Koschel wrote: > >> 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.
|  |