From: Julia Cartwright <julia@ni.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Rafal Ozieblo <rafalo@cadence.com>,
Nicolas Ferre <nicolas.ferre@microchip.com>,
<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<kbuild-all@01.org>
Subject: [PATCH 1/2] net: macb: reduce scope of rx_fs_lock-protected regions
Date: Tue, 5 Dec 2017 14:17:11 -0600 [thread overview]
Message-ID: <20171205201711.GA18445@jcartwri.amer.corp.natinst.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1712020800010.2168@hadrien>
Commit ae8223de3df5 ("net: macb: Added support for RX filtering")
introduces a lock, rx_fs_lock which is intended to protect the list of
rx_flow items and synchronize access to the hardware rx filtering
registers.
However, the region protected by this lock is overscoped, unnecessarily
including things like slab allocation. Reduce this lock scope to only
include operations which must be performed atomically: list traversal,
addition, and removal, and hitting the macb filtering registers.
This fixes the use of kmalloc w/ GFP_KERNEL in atomic context.
Fixes: ae8223de3df5 ("net: macb: Added support for RX filtering")
Cc: Rafal Ozieblo <rafalo@cadence.com>
Cc: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Julia Cartwright <julia@ni.com>
---
While Julia Lawall's cocci-generated patch fixes the problem, the right
solution is to obviate the problem altogether.
Thanks,
The Other Julia
drivers/net/ethernet/cadence/macb_main.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c5fa87cdc6c4..e7ef104a077d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2796,6 +2796,7 @@ static int gem_add_flow_filter(struct net_device *netdev,
struct macb *bp = netdev_priv(netdev);
struct ethtool_rx_flow_spec *fs = &cmd->fs;
struct ethtool_rx_fs_item *item, *newfs;
+ unsigned long flags;
int ret = -EINVAL;
bool added = false;
@@ -2811,6 +2812,8 @@ static int gem_add_flow_filter(struct net_device *netdev,
htonl(fs->h_u.tcp_ip4_spec.ip4dst),
htons(fs->h_u.tcp_ip4_spec.psrc), htons(fs->h_u.tcp_ip4_spec.pdst));
+ 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);
@@ -2837,9 +2840,11 @@ static int gem_add_flow_filter(struct net_device *netdev,
if (netdev->features & NETIF_F_NTUPLE)
gem_enable_flow_filters(bp, 1);
+ spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
return 0;
err:
+ spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
kfree(newfs);
return ret;
}
@@ -2850,9 +2855,14 @@ static int gem_del_flow_filter(struct net_device *netdev,
struct macb *bp = netdev_priv(netdev);
struct ethtool_rx_fs_item *item;
struct ethtool_rx_flow_spec *fs;
+ unsigned long flags;
- if (list_empty(&bp->rx_fs_list.list))
+ 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) {
@@ -2869,12 +2879,14 @@ static int gem_del_flow_filter(struct net_device *netdev,
gem_writel_n(bp, SCRT2, fs->location, 0);
list_del(&item->list);
- kfree(item);
bp->rx_fs_list.count--;
+ spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
+ kfree(item);
return 0;
}
}
+ spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
return -EINVAL;
}
@@ -2943,11 +2955,8 @@ static int gem_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
{
struct macb *bp = netdev_priv(netdev);
- unsigned long flags;
int ret;
- spin_lock_irqsave(&bp->rx_fs_lock, flags);
-
switch (cmd->cmd) {
case ETHTOOL_SRXCLSRLINS:
if ((cmd->fs.location >= bp->max_tuples)
@@ -2966,7 +2975,6 @@ static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
ret = -EOPNOTSUPP;
}
- spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
return ret;
}
--
2.14.2
next prev parent reply other threads:[~2017-12-05 20:17 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-02 7:01 [PATCH v2] net: macb: change GFP_KERNEL to GFP_ATOMIC Julia Lawall
2017-12-05 16:27 ` David Miller
2017-12-05 20:17 ` Julia Cartwright [this message]
2017-12-05 20:18 ` [PATCH 2/2] net: macb: kill useless use of list_empty() Julia Cartwright
2017-12-05 21:28 ` Nicolas Ferre
2017-12-05 21:26 ` [PATCH 1/2] net: macb: reduce scope of rx_fs_lock-protected regions Nicolas Ferre
2017-12-05 23:00 ` David Miller
2017-12-06 0:02 ` [PATCH v2 0/3] macb rx filter cleanups Julia Cartwright
2017-12-06 0:02 ` [PATCH v2 1/3] net: macb: kill useless use of list_empty() Julia Cartwright
2017-12-06 0:02 ` [PATCH v2 2/3] net: macb: reduce scope of rx_fs_lock-protected regions Julia Cartwright
2017-12-06 0:02 ` [PATCH v2 3/3] net: macb: change GFP_ATOMIC to GFP_KERNEL Julia Cartwright
2017-12-06 5:49 ` Julia Lawall
2017-12-06 1:08 ` [PATCH v2 0/3] macb rx filter cleanups David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171205201711.GA18445@jcartwri.amer.corp.natinst.com \
--to=julia@ni.com \
--cc=julia.lawall@lip6.fr \
--cc=kbuild-all@01.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nicolas.ferre@microchip.com \
--cc=rafalo@cadence.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®