Am 09.02.2011 00:04, schrieb David Miller:
> From: David Miller <davem@xxxxxxxxxxxxx>
> Date: Tue, 08 Feb 2011 00:07:21 -0800 (PST)
>
> [ Eric B., CC:'ing you so that you are aware of the init network
> namespace leak that's fixed as a side effect of this change. ]
>
>
>> From: Knut Petersen <Knut_Petersen@xxxxxxxxxxx>
>> Date: Tue, 08 Feb 2011 08:51:22 +0100
>>
>>
>>> I bisected the problem with the following result:
>>>
>>> aa9421041128abb4d269ee1dc502ff65fb3b7d69 is the first bad commit
>>> commit aa9421041128abb4d269ee1dc502ff65fb3b7d69
>>> Author: Changli Gao <xiaosuo@xxxxxxxxx>
>>> Date: Sat Dec 4 02:31:41 2010 +0000
>>>
>>> net: init ingress queue
>>>
>>> The dev field of ingress queue is forgot to initialized, then NULL
>>> pointer dereference happens in qdisc_alloc().
>>>
>>> Move inits of tx queues to netif_alloc_netdev_queues().
>>>
>>> Signed-off-by: Changli Gao <xiaosuo@xxxxxxxxx>
>>> Acked-by: Eric Dumazet <eric.dumazet@xxxxxxxxx>
>>> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
>>>
>>> :040000 040000 dcbb6ab41c4308cba1bc6823d200dcf92aa402d8
>>> b5e190ec681d26ffe62d1d0214c4ef77b8034189 M net
>>>
>> Indeed, this initialization is now too early for the sake
>> of getting the lockdep bits right. The problem is that at
>> the point in which we call netif_alloc_netdev_queue() we
>> haven't initialized dev->type yet, it is therefore always
>> zero when we setup the lockdep class for ->_xmit_lock.
>>
> Ok, this should fix it, please test it out for me.
>
> Thanks!
>
Yes, that patch does solve the problem, and I do not see
any negative effects.
Thanks!
Knut
> --------------------
> net: Fix lockdep regression caused by initializing netdev queues too early.
>
> In commit aa9421041128abb4d269ee1dc502ff65fb3b7d69 ("net: init ingress
> queue") we moved the allocation and lock initialization of the queues
> into alloc_netdev_mq() since register_netdevice() is way too late.
>
> The problem is that dev->type is not setup until the setup()
> callback is invoked by alloc_netdev_mq(), and the dev->type is
> what determines the lockdep class to use for the locks in the
> queues.
>
> Fix this by doing the queue allocation after the setup() callback
> runs.
>
> This is safe because the setup() callback is not allowed to make any
> state changes that need to be undone on error (memory allocations,
> etc.). It may, however, make state changes that are undone by
> free_netdev() (such as netif_napi_add(), which is done by the
> ipoib driver's setup routine).
>
> The previous code also leaked a reference to the &init_net namespace
> object on RX/TX queue allocation failures.
>
> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
> ---
> net/core/dev.c | 27 ++++++++++++++++-----------
> 1 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b6d0bf8..8e726cb 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5660,30 +5660,35 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
>
> dev_net_set(dev, &init_net);
>
> + dev->gso_max_size = GSO_MAX_SIZE;
> +
> + INIT_LIST_HEAD(&dev->ethtool_ntuple_list.list);
> + dev->ethtool_ntuple_list.count = 0;
> + INIT_LIST_HEAD(&dev->napi_list);
> + INIT_LIST_HEAD(&dev->unreg_list);
> + INIT_LIST_HEAD(&dev->link_watch_list);
> + dev->priv_flags = IFF_XMIT_DST_RELEASE;
> + setup(dev);
> +
> dev->num_tx_queues = txqs;
> dev->real_num_tx_queues = txqs;
> if (netif_alloc_netdev_queues(dev))
> - goto free_pcpu;
> + goto free_all;
>
> #ifdef CONFIG_RPS
> dev->num_rx_queues = rxqs;
> dev->real_num_rx_queues = rxqs;
> if (netif_alloc_rx_queues(dev))
> - goto free_pcpu;
> + goto free_all;
> #endif
>
> - dev->gso_max_size = GSO_MAX_SIZE;
> -
> - INIT_LIST_HEAD(&dev->ethtool_ntuple_list.list);
> - dev->ethtool_ntuple_list.count = 0;
> - INIT_LIST_HEAD(&dev->napi_list);
> - INIT_LIST_HEAD(&dev->unreg_list);
> - INIT_LIST_HEAD(&dev->link_watch_list);
> - dev->priv_flags = IFF_XMIT_DST_RELEASE;
> - setup(dev);
> strcpy(dev->name, name);
> return dev;
>
> +free_all:
> + free_netdev(dev);
> + return NULL;
> +
> free_pcpu:
> free_percpu(dev->pcpu_refcnt);
> kfree(dev->_tx);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ppp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
