Hi,
sparse complains the following:
drivers/net/ethernet/freescale/fec_main.c:2835:12: warning: context
imbalance in 'fec_set_features' - different lock contexts for basic
block
The code looks like this:
static int fec_set_features(struct net_device *netdev,
netdev_features_t features)
{
struct fec_enet_private *fep = netdev_priv(netdev);
netdev_features_t changed = features ^ netdev->features;
/* Quiesce the device if necessary */
if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
napi_disable(&fep->napi);
netif_tx_lock_bh(netdev);
fec_stop(netdev);
}
netdev->features = features;
/* Receive checksum has been changed */
if (changed & NETIF_F_RXCSUM) {
if (features & NETIF_F_RXCSUM)
fep->csum_flags |= FLAG_RX_CSUM_ENABLED;
else
fep->csum_flags &= ~FLAG_RX_CSUM_ENABLED;
}
/* Resume the device after updates */
if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
fec_restart(netdev);
netif_tx_wake_all_queues(netdev);
netif_tx_unlock_bh(netdev);
napi_enable(&fep->napi);
}
return 0;
}
What would be the proper way to fix this warning?
Thanks,
Fabio Estevam
From: Fabio Estevam <festevam@gmail.com> Sent: Thursday, October 02, 2014 8:16 AM
To: Li Frank-B20596; Duan Fugang-B38611
Cc: David S. Miller; netdev@vger.kernel.org; linux-sparse@vger.kernel.org
Subject: fec_main: context imbalance in 'fec_set_features'
Hi,
sparse complains the following:
drivers/net/ethernet/freescale/fec_main.c:2835:12: warning: context
imbalance in 'fec_set_features' - different lock contexts for basic block
The code looks like this:
static int fec_set_features(struct net_device *netdev,
netdev_features_t features)
{
struct fec_enet_private *fep = netdev_priv(netdev);
netdev_features_t changed = features ^ netdev->features;
/* Quiesce the device if necessary */
if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
napi_disable(&fep->napi);
netif_tx_lock_bh(netdev);
fec_stop(netdev);
}
netdev->features = features;
/* Receive checksum has been changed */
if (changed & NETIF_F_RXCSUM) {
if (features & NETIF_F_RXCSUM)
fep->csum_flags |= FLAG_RX_CSUM_ENABLED;
else
fep->csum_flags &= ~FLAG_RX_CSUM_ENABLED;
}
/* Resume the device after updates */
if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
fec_restart(netdev);
netif_tx_wake_all_queues(netdev);
netif_tx_unlock_bh(netdev);
napi_enable(&fep->napi);
}
return 0;
}
What would be the proper way to fix this warning?
Thanks,
Fabio Estevam
Hi, Fabio,
I run sparse and cannot get any warning.
1. git clone git://git.kernel.org/pub/scm/devel/sparse/sparse.git
make
make install
2. export PATH=$PATH:~/bin/
3. make kernel: make -j16 ARCH=arm CROSS_COMPILE=arm-poky-linux-gnueabi- zImage C=2
Commit ID on David net tree: 28b7deae75642c51f097391765fd39ff0dd6ce95
The result:
CHECK fs/lockd/svc4proc.c
CHECK drivers/of/platform.c
CHECK drivers/input/touchscreen/tsc2007.c
CHECK drivers/mtd/nand/nand_timings.c
CHECK drivers/pci/probe.c
CHECK drivers/pci/host-bridge.c
CHECK drivers/pci/remove.c
drivers/mtd/nand/nand_timings.c:45:29: warning: constant 250000000000 is so big it is long long
CHECK drivers/mtd/nand/nand_ecc.c
CHECK drivers/mtd/nand/nand_ids.c
CHECK drivers/net/ethernet/freescale/fec_main.c
CHECK drivers/of/fdt.c
CHECK drivers/of/fdt_address.c
CHECK drivers/pci/pci.c
CHECK drivers/mtd/nand/mxc_nand.c
CHECK fs/notify/fsnotify.c
CHECK fs/notify/notification.c
CHECK fs/notify/group.c
CHECK fs/notify/inode_mark.c
CHECK drivers/net/ethernet/freescale/fec_ptp.c
CHECK fs/notify/mark.c
CHECK fs/notify/vfsmount_mark.c
drivers/pci/pci.c:35:5: warning: symbol 'isa_dma_bridge_buggy' was not declared. Should it be static?
drivers/pci/pci.c:4392:1: warning: symbol 'bus_attr_resource_alignment' was not declared. Should it be static?
CHECK fs/notify/fdinfo.c
CHECK drivers/pci/pci-driver.c
CHECK drivers/pci/search.c
CHECK drivers/pci/pci-sysfs.c
CHECK drivers/pci/rom.c
CHECK drivers/pci/setup-res.c
Regards,
Andy
Hi Fugang,
On Thu, Oct 9, 2014 at 6:55 AM, fugang.duan@freescale.com
[off-list ref] wrote:
I run sparse and cannot get any warning.
Try it on linux-next with the following command:
make -j4 C=1 2>fect.txt drivers/net/ethernet/freescale/fec_main.o
cat v6.txt
drivers/net/ethernet/freescale/fec_main.c:2916:12: warning: context
imbalance in 'fec_set_features' - different lock contexts for basic
block
On Thu, Oct 2, 2014 at 8:15 AM, Fabio Estevam [off-list ref] wrote:
Hi,
sparse complains the following:
drivers/net/ethernet/freescale/fec_main.c:2835:12: warning: context
imbalance in 'fec_set_features' - different lock contexts for basic
block
That is expected to receive warnings. Sparse can't really tell the lock
and unlock are actually on the same condition and it never change.
The code looks like this:
static int fec_set_features(struct net_device *netdev,
netdev_features_t features)
{
struct fec_enet_private *fep = netdev_priv(netdev);
netdev_features_t changed = features ^ netdev->features;
/* Quiesce the device if necessary */
if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
e.g. netdev is running here, you take the lock.
napi_disable(&fep->napi);
netif_tx_lock_bh(netdev);
fec_stop(netdev);
}
netdev->features = features;quoted
/* Resume the device after updates */
if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
Then there is race some one shutdown the netdev before the code hits
here (is it possible? it is hard to tell from this source alone.)
Then the unlock is skipped.
fec_restart(netdev);
netif_tx_wake_all_queues(netdev);
Sparse is complaining, there exist a code path, from execution flow
point of view, (without considering the data flow analyze), there exists a
code path lock and unlock are not balanced.
It is possible that warning code path is not actually executable
due to data flow reason(e.g. "changed" never change during the same function).
But sparse did not have data flow analyze to know that.
If you move the code, e.g. the code inside the lock area into a function.
You can write:
if (need_lock) {
lock();
do_something_real();
unlock();
} else {
do_something_real();
}
That way, sparse don't see such a code path can trigger lock unbalanced.
Chris