| Message ID | 20171205201830.GB18445@jcartwri.amer.corp.natinst.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [1/2] net: macb: reduce scope of rx_fs_lock-protected regions | expand |
On 05/12/2017 at 21:18, Julia Cartwright wrote: > The list_for_each_entry() macro already handles the case where the list > is empty (by not executing the loop body). It's not necessary to handle > this case specially, so stop doing so. > > Cc: Rafal Ozieblo <rafalo@cadence.com> > Signed-off-by: Julia Cartwright <julia@ni.com> > --- > This is an additional cleanup patch found when looking at this code. > > Julia Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> Thanks > > drivers/net/ethernet/cadence/macb_main.c | 34 ++++++++++++-------------------- > 1 file changed, 13 insertions(+), 21 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > index e7ef104a077d..3643c6ad2322 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -2815,25 +2815,22 @@ static int gem_add_flow_filter(struct net_device *netdev, > spin_lock_irqsave(&bp->rx_fs_lock, flags); > > /* find correct place to add in list */ > - if (list_empty(&bp->rx_fs_list.list)) > - list_add(&newfs->list, &bp->rx_fs_list.list); > - else { > - list_for_each_entry(item, &bp->rx_fs_list.list, list) { > - if (item->fs.location > newfs->fs.location) { > - list_add_tail(&newfs->list, &item->list); > - added = true; > - break; > - } else if (item->fs.location == fs->location) { > - netdev_err(netdev, "Rule not added: location %d not free!\n", > - fs->location); > - ret = -EBUSY; > - goto err; > - } > + list_for_each_entry(item, &bp->rx_fs_list.list, list) { > + if (item->fs.location > newfs->fs.location) { > + list_add_tail(&newfs->list, &item->list); > + added = true; > + break; > + } else if (item->fs.location == fs->location) { > + netdev_err(netdev, "Rule not added: location %d not free!\n", > + fs->location); > + ret = -EBUSY; > + goto err; > } > - if (!added) > - list_add_tail(&newfs->list, &bp->rx_fs_list.list); > } > > + if (!added) > + list_add_tail(&newfs->list, &bp->rx_fs_list.list); > + > gem_prog_cmp_regs(bp, fs); > bp->rx_fs_list.count++; > /* enable filtering if NTUPLE on */ > @@ -2859,11 +2856,6 @@ static int gem_del_flow_filter(struct net_device *netdev, > > spin_lock_irqsave(&bp->rx_fs_lock, flags); > > - if (list_empty(&bp->rx_fs_list.list)) { > - spin_unlock_irqrestore(&bp->rx_fs_lock, flags); > - return -EINVAL; > - } > - > list_for_each_entry(item, &bp->rx_fs_list.list, list) { > if (item->fs.location == cmd->fs.location) { > /* disable screener regs for the flow entry */ >
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index e7ef104a077d..3643c6ad2322 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -2815,25 +2815,22 @@ static int gem_add_flow_filter(struct net_device *netdev, spin_lock_irqsave(&bp->rx_fs_lock, flags); /* find correct place to add in list */ - if (list_empty(&bp->rx_fs_list.list)) - list_add(&newfs->list, &bp->rx_fs_list.list); - else { - list_for_each_entry(item, &bp->rx_fs_list.list, list) { - if (item->fs.location > newfs->fs.location) { - list_add_tail(&newfs->list, &item->list); - added = true; - break; - } else if (item->fs.location == fs->location) { - netdev_err(netdev, "Rule not added: location %d not free!\n", - fs->location); - ret = -EBUSY; - goto err; - } + list_for_each_entry(item, &bp->rx_fs_list.list, list) { + if (item->fs.location > newfs->fs.location) { + list_add_tail(&newfs->list, &item->list); + added = true; + break; + } else if (item->fs.location == fs->location) { + netdev_err(netdev, "Rule not added: location %d not free!\n", + fs->location); + ret = -EBUSY; + goto err; } - if (!added) - list_add_tail(&newfs->list, &bp->rx_fs_list.list); } + if (!added) + list_add_tail(&newfs->list, &bp->rx_fs_list.list); + gem_prog_cmp_regs(bp, fs); bp->rx_fs_list.count++; /* enable filtering if NTUPLE on */ @@ -2859,11 +2856,6 @@ static int gem_del_flow_filter(struct net_device *netdev, spin_lock_irqsave(&bp->rx_fs_lock, flags); - if (list_empty(&bp->rx_fs_list.list)) { - spin_unlock_irqrestore(&bp->rx_fs_lock, flags); - return -EINVAL; - } - list_for_each_entry(item, &bp->rx_fs_list.list, list) { if (item->fs.location == cmd->fs.location) { /* disable screener regs for the flow entry */
The list_for_each_entry() macro already handles the case where the list is empty (by not executing the loop body). It's not necessary to handle this case specially, so stop doing so. Cc: Rafal Ozieblo <rafalo@cadence.com> Signed-off-by: Julia Cartwright <julia@ni.com> --- This is an additional cleanup patch found when looking at this code. Julia drivers/net/ethernet/cadence/macb_main.c | 34 ++++++++++++-------------------- 1 file changed, 13 insertions(+), 21 deletions(-)