From patchwork Mon Apr 30 17:15:47 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roland Dreier X-Patchwork-Id: 155927 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 11A71B6FA8 for ; Tue, 1 May 2012 03:15:56 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753988Ab2D3RPy (ORCPT ); Mon, 30 Apr 2012 13:15:54 -0400 Received: from na3sys010aog102.obsmtp.com ([74.125.245.72]:34940 "HELO na3sys010aog102.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753211Ab2D3RPx (ORCPT ); Mon, 30 Apr 2012 13:15:53 -0400 Received: from mail-pb0-f43.google.com ([209.85.160.43]) (using TLSv1) by na3sys010aob102.postini.com ([74.125.244.12]) with SMTP ID DSNKT57IyJp4OG7i4a65xkWFolVkuuq2kDNL@postini.com; Mon, 30 Apr 2012 10:15:52 PDT Received: by pbcwz7 with SMTP id wz7so1749798pbc.16 for ; Mon, 30 Apr 2012 10:15:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=purestorage.com; s=google; h=sender:from:to:cc:subject:date:message-id:x-mailer; bh=jteNKcbcbwP9sDosSows6Vd3DbOuCIt7djjBiuaQ/qo=; b=f4XclLvwg0ULwopCd14PRN+pxxXMTP2Z/Dey/Xknkdun91LOzSuLYS8xDC2O6OOWm7 83ZOX95zi9ak2sG9Zrk6DYP6v+KeXMWo9ePHGNi0HyX4xGndJixoAWlv7vNHDPuudxl8 con1nTtA8ZMtXag8UIPkWKgPM9YAQBl0kG46U= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:x-mailer :x-gm-message-state; bh=jteNKcbcbwP9sDosSows6Vd3DbOuCIt7djjBiuaQ/qo=; b=e5LGPAkNhLEkzP3rXnijmHhPtIm1PfexNq9JX4pJMs8aUsLVDtS+5VZLXgkCPYG6z4 NlG68oDgr7CWGlwafKTfqwiKK7bU1K0ZVmbm1InY9yMxufbCE3u+4tRpRP2+MdNZN9yK gEflVkkFh7KCi1xJUJMjw3hGz+Ws3bXb4FLh51qPdazrffYqu1gJCdaO+x2cojcGmUw4 Is+BBv9NRMMkM/U9QoEXgRawgjXx7WLhfrYZWADSOuL2xcF6WLlXmo7sWXg2cMhyG6xj 9sP34KFnh/TbtQDnU8QTQ5zFirUfnzSWfVrgiyCdg5miMgikNj3eKbM4y1t0Sr5qdieD zueA== Received: by 10.68.202.130 with SMTP id ki2mr5137792pbc.52.1335806151459; Mon, 30 Apr 2012 10:15:51 -0700 (PDT) Received: from roland-t410s.purestorage.com ([216.200.155.2]) by mx.google.com with ESMTPS id vq9sm5899715pbc.18.2012.04.30.10.15.50 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 30 Apr 2012 10:15:50 -0700 (PDT) From: Roland Dreier To: Divy Le Ray , "David S. Miller" Cc: netdev@vger.kernel.org, Roland Dreier Subject: [PATCH] cxgb3: Don't call cxgb_vlan_mode until q locks are initialized Date: Mon, 30 Apr 2012 10:15:47 -0700 Message-Id: <1335806147-17310-1-git-send-email-roland@kernel.org> X-Mailer: git-send-email 1.7.9.5 X-Gm-Message-State: ALoCoQl/Dlwp6Z7xumwq3weTijNHMJFA77qAMrRfUd4zR+ba6kwfoaJ18vjGcKsr762eOi3xvbsm Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Roland Dreier The driver calls cxgb_vlan_mode() from init_one(). This calls into synchronize_rx(), which locks all the q locks, but the q locks are not initialized until cxgb_up() -> setup_sge_qsets(). So move the call to cxgb_vlan_mode() into cxgb_up(), after the call to setup_sge_qsets(). We also move the body of these functions up higher to avoid having to a forward declaration. This was found because of the lockdep warning: INFO: trying to register non-static key. the code is fine but needs lockdep annotation. turning off the locking correctness validator. Pid: 323, comm: work_for_cpu Not tainted 3.4.0-rc5 #28 Call Trace: [] register_lock_class+0x108/0x2d0 [] __lock_acquire+0xd3/0xd06 [] lock_acquire+0xbf/0xfe [] _raw_spin_lock_irq+0x36/0x45 [] cxgb_vlan_mode+0x96/0xcb [cxgb3] [] init_one+0x8c4/0x980 [cxgb3] [] local_pci_probe+0x3f/0x70 [] do_work_for_cpu+0x10/0x22 [] kthread+0xa1/0xa9 [] kernel_thread_helper+0x4/0x10 Contrary to what lockdep says, the code is not fine: we are locking an uninitialized spinlock. Signed-off-by: Roland Dreier --- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 92 +++++++++++------------ 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c index 63bfdd1..abb6ce7 100644 --- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c +++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c @@ -1150,6 +1150,48 @@ release_tpsram: } /** + * t3_synchronize_rx - wait for current Rx processing on a port to complete + * @adap: the adapter + * @p: the port + * + * Ensures that current Rx processing on any of the queues associated with + * the given port completes before returning. We do this by acquiring and + * releasing the locks of the response queues associated with the port. + */ +static void t3_synchronize_rx(struct adapter *adap, const struct port_info *p) +{ + int i; + + for (i = p->first_qset; i < p->first_qset + p->nqsets; i++) { + struct sge_rspq *q = &adap->sge.qs[i].rspq; + + spin_lock_irq(&q->lock); + spin_unlock_irq(&q->lock); + } +} + +static void cxgb_vlan_mode(struct net_device *dev, netdev_features_t features) +{ + struct port_info *pi = netdev_priv(dev); + struct adapter *adapter = pi->adapter; + + if (adapter->params.rev > 0) { + t3_set_vlan_accel(adapter, 1 << pi->port_id, + features & NETIF_F_HW_VLAN_RX); + } else { + /* single control for all ports */ + unsigned int i, have_vlans = features & NETIF_F_HW_VLAN_RX; + + for_each_port(adapter, i) + have_vlans |= + adapter->port[i]->features & NETIF_F_HW_VLAN_RX; + + t3_set_vlan_accel(adapter, 1, have_vlans); + } + t3_synchronize_rx(adapter, pi); +} + +/** * cxgb_up - enable the adapter * @adapter: adapter being enabled * @@ -1161,7 +1203,7 @@ release_tpsram: */ static int cxgb_up(struct adapter *adap) { - int err; + int i, err; if (!(adap->flags & FULL_INIT_DONE)) { err = t3_check_fw_version(adap); @@ -1198,6 +1240,9 @@ static int cxgb_up(struct adapter *adap) if (err) goto out; + for_each_port(adap, i) + cxgb_vlan_mode(adap->port[i], adap->port[i]->features); + setup_rss(adap); if (!(adap->flags & NAPI_INIT)) init_napi(adap); @@ -2508,48 +2553,6 @@ static int cxgb_set_mac_addr(struct net_device *dev, void *p) return 0; } -/** - * t3_synchronize_rx - wait for current Rx processing on a port to complete - * @adap: the adapter - * @p: the port - * - * Ensures that current Rx processing on any of the queues associated with - * the given port completes before returning. We do this by acquiring and - * releasing the locks of the response queues associated with the port. - */ -static void t3_synchronize_rx(struct adapter *adap, const struct port_info *p) -{ - int i; - - for (i = p->first_qset; i < p->first_qset + p->nqsets; i++) { - struct sge_rspq *q = &adap->sge.qs[i].rspq; - - spin_lock_irq(&q->lock); - spin_unlock_irq(&q->lock); - } -} - -static void cxgb_vlan_mode(struct net_device *dev, netdev_features_t features) -{ - struct port_info *pi = netdev_priv(dev); - struct adapter *adapter = pi->adapter; - - if (adapter->params.rev > 0) { - t3_set_vlan_accel(adapter, 1 << pi->port_id, - features & NETIF_F_HW_VLAN_RX); - } else { - /* single control for all ports */ - unsigned int i, have_vlans = features & NETIF_F_HW_VLAN_RX; - - for_each_port(adapter, i) - have_vlans |= - adapter->port[i]->features & NETIF_F_HW_VLAN_RX; - - t3_set_vlan_accel(adapter, 1, have_vlans); - } - t3_synchronize_rx(adapter, pi); -} - static netdev_features_t cxgb_fix_features(struct net_device *dev, netdev_features_t features) { @@ -3353,9 +3356,6 @@ static int __devinit init_one(struct pci_dev *pdev, err = sysfs_create_group(&adapter->port[0]->dev.kobj, &cxgb3_attr_group); - for_each_port(adapter, i) - cxgb_vlan_mode(adapter->port[i], adapter->port[i]->features); - print_port_info(adapter, ai); return 0;