Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

qed: Fix scheduling in a tasklet while getting stats

Here we've got to a situation when tasklet called usleep_range() in PTT
acquire logic, thus welcome to the "scheduling while atomic" BUG().

BUG: scheduling while atomic: swapper/24/0/0x00000100

[<ffffffffb41c6199>] schedule+0x29/0x70
[<ffffffffb41c5512>] schedule_hrtimeout_range_clock+0xb2/0x150
[<ffffffffb41c55c3>] schedule_hrtimeout_range+0x13/0x20
[<ffffffffb41c3bcf>] usleep_range+0x4f/0x70
[<ffffffffc08d3e58>] qed_ptt_acquire+0x38/0x100 [qed]
[<ffffffffc08eac48>] _qed_get_vport_stats+0x458/0x580 [qed]
[<ffffffffc08ead8c>] qed_get_vport_stats+0x1c/0xd0 [qed]
[<ffffffffc08dffd3>] qed_get_protocol_stats+0x93/0x100 [qed]
qed_mcp_send_protocol_stats
case MFW_DRV_MSG_GET_LAN_STATS:
case MFW_DRV_MSG_GET_FCOE_STATS:
case MFW_DRV_MSG_GET_ISCSI_STATS:
case MFW_DRV_MSG_GET_RDMA_STATS:
[<ffffffffc08e36d8>] qed_mcp_handle_events+0x2d8/0x890 [qed]
qed_int_assertion
qed_int_attentions
[<ffffffffc08d9490>] qed_int_sp_dpc+0xa50/0xdc0 [qed]
[<ffffffffb3aa7623>] tasklet_action+0x83/0x140
[<ffffffffb41d9125>] __do_softirq+0x125/0x2bb
[<ffffffffb41d560c>] call_softirq+0x1c/0x30
[<ffffffffb3a30645>] do_softirq+0x65/0xa0
[<ffffffffb3aa78d5>] irq_exit+0x105/0x110
[<ffffffffb41d8996>] do_IRQ+0x56/0xf0

Fix this by making caller to provide the context whether it could be in
atomic context flow or not when getting stats from QED driver.
QED driver based on the context provided decide to schedule out or not
when acquiring the PTT BAR window.

We faced the BUG_ON() while getting vport stats, but according to the
code same issue could happen for fcoe and iscsi statistics as well, so
fixing them too.

Fixes: 6c75424612a7 ("qed: Add support for NCSI statistics.")
Fixes: 1e128c81290a ("qed: Add support for hardware offloaded FCoE.")
Fixes: 2f2b2614e893 ("qed: Provide iSCSI statistics to management")
Cc: Sudarsana Kalluru <skalluru@marvell.com>
Cc: David Miller <davem@davemloft.net>
Cc: Manish Chopra <manishc@marvell.com>

Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Konstantin Khorenko and committed by
David S. Miller
e346e231 8d7ae22a

+128 -26
+16
drivers/net/ethernet/qlogic/qed/qed_dev_api.h
··· 194 194 struct qed_ptt *qed_ptt_acquire(struct qed_hwfn *p_hwfn); 195 195 196 196 /** 197 + * qed_ptt_acquire_context(): Allocate a PTT window honoring the context 198 + * atomicy. 199 + * 200 + * @p_hwfn: HW device data. 201 + * @is_atomic: Hint from the caller - if the func can sleep or not. 202 + * 203 + * Context: The function should not sleep in case is_atomic == true. 204 + * Return: struct qed_ptt. 205 + * 206 + * Should be called at the entry point to the driver 207 + * (at the beginning of an exported function). 208 + */ 209 + struct qed_ptt *qed_ptt_acquire_context(struct qed_hwfn *p_hwfn, 210 + bool is_atomic); 211 + 212 + /** 197 213 * qed_ptt_release(): Release PTT Window. 198 214 * 199 215 * @p_hwfn: HW device data.
+14 -5
drivers/net/ethernet/qlogic/qed/qed_fcoe.c
··· 693 693 } 694 694 695 695 static int qed_fcoe_get_stats(struct qed_hwfn *p_hwfn, 696 - struct qed_fcoe_stats *p_stats) 696 + struct qed_fcoe_stats *p_stats, 697 + bool is_atomic) 697 698 { 698 699 struct qed_ptt *p_ptt; 699 700 700 701 memset(p_stats, 0, sizeof(*p_stats)); 701 702 702 - p_ptt = qed_ptt_acquire(p_hwfn); 703 + p_ptt = qed_ptt_acquire_context(p_hwfn, is_atomic); 703 704 704 705 if (!p_ptt) { 705 706 DP_ERR(p_hwfn, "Failed to acquire ptt\n"); ··· 974 973 QED_SPQ_MODE_EBLOCK, NULL); 975 974 } 976 975 976 + static int qed_fcoe_stats_context(struct qed_dev *cdev, 977 + struct qed_fcoe_stats *stats, 978 + bool is_atomic) 979 + { 980 + return qed_fcoe_get_stats(QED_AFFIN_HWFN(cdev), stats, is_atomic); 981 + } 982 + 977 983 static int qed_fcoe_stats(struct qed_dev *cdev, struct qed_fcoe_stats *stats) 978 984 { 979 - return qed_fcoe_get_stats(QED_AFFIN_HWFN(cdev), stats); 985 + return qed_fcoe_stats_context(cdev, stats, false); 980 986 } 981 987 982 988 void qed_get_protocol_stats_fcoe(struct qed_dev *cdev, 983 - struct qed_mcp_fcoe_stats *stats) 989 + struct qed_mcp_fcoe_stats *stats, 990 + bool is_atomic) 984 991 { 985 992 struct qed_fcoe_stats proto_stats; 986 993 987 994 /* Retrieve FW statistics */ 988 995 memset(&proto_stats, 0, sizeof(proto_stats)); 989 - if (qed_fcoe_stats(cdev, &proto_stats)) { 996 + if (qed_fcoe_stats_context(cdev, &proto_stats, is_atomic)) { 990 997 DP_VERBOSE(cdev, QED_MSG_STORAGE, 991 998 "Failed to collect FCoE statistics\n"); 992 999 return;
+15 -2
drivers/net/ethernet/qlogic/qed/qed_fcoe.h
··· 28 28 void qed_fcoe_setup(struct qed_hwfn *p_hwfn); 29 29 30 30 void qed_fcoe_free(struct qed_hwfn *p_hwfn); 31 + /** 32 + * qed_get_protocol_stats_fcoe(): Fills provided statistics 33 + * struct with statistics. 34 + * 35 + * @cdev: Qed dev pointer. 36 + * @stats: Points to struct that will be filled with statistics. 37 + * @is_atomic: Hint from the caller - if the func can sleep or not. 38 + * 39 + * Context: The function should not sleep in case is_atomic == true. 40 + * Return: Void. 41 + */ 31 42 void qed_get_protocol_stats_fcoe(struct qed_dev *cdev, 32 - struct qed_mcp_fcoe_stats *stats); 43 + struct qed_mcp_fcoe_stats *stats, 44 + bool is_atomic); 33 45 #else /* CONFIG_QED_FCOE */ 34 46 static inline int qed_fcoe_alloc(struct qed_hwfn *p_hwfn) 35 47 { ··· 52 40 static inline void qed_fcoe_free(struct qed_hwfn *p_hwfn) {} 53 41 54 42 static inline void qed_get_protocol_stats_fcoe(struct qed_dev *cdev, 55 - struct qed_mcp_fcoe_stats *stats) 43 + struct qed_mcp_fcoe_stats *stats, 44 + bool is_atomic) 56 45 { 57 46 } 58 47 #endif /* CONFIG_QED_FCOE */
+22 -4
drivers/net/ethernet/qlogic/qed/qed_hw.c
··· 23 23 #include "qed_reg_addr.h" 24 24 #include "qed_sriov.h" 25 25 26 - #define QED_BAR_ACQUIRE_TIMEOUT 1000 26 + #define QED_BAR_ACQUIRE_TIMEOUT_USLEEP_CNT 1000 27 + #define QED_BAR_ACQUIRE_TIMEOUT_USLEEP 1000 28 + #define QED_BAR_ACQUIRE_TIMEOUT_UDELAY_CNT 100000 29 + #define QED_BAR_ACQUIRE_TIMEOUT_UDELAY 10 27 30 28 31 /* Invalid values */ 29 32 #define QED_BAR_INVALID_OFFSET (cpu_to_le32(-1)) ··· 88 85 89 86 struct qed_ptt *qed_ptt_acquire(struct qed_hwfn *p_hwfn) 90 87 { 88 + return qed_ptt_acquire_context(p_hwfn, false); 89 + } 90 + 91 + struct qed_ptt *qed_ptt_acquire_context(struct qed_hwfn *p_hwfn, bool is_atomic) 92 + { 91 93 struct qed_ptt *p_ptt; 92 - unsigned int i; 94 + unsigned int i, count; 95 + 96 + if (is_atomic) 97 + count = QED_BAR_ACQUIRE_TIMEOUT_UDELAY_CNT; 98 + else 99 + count = QED_BAR_ACQUIRE_TIMEOUT_USLEEP_CNT; 93 100 94 101 /* Take the free PTT from the list */ 95 - for (i = 0; i < QED_BAR_ACQUIRE_TIMEOUT; i++) { 102 + for (i = 0; i < count; i++) { 96 103 spin_lock_bh(&p_hwfn->p_ptt_pool->lock); 97 104 98 105 if (!list_empty(&p_hwfn->p_ptt_pool->free_list)) { ··· 118 105 } 119 106 120 107 spin_unlock_bh(&p_hwfn->p_ptt_pool->lock); 121 - usleep_range(1000, 2000); 108 + 109 + if (is_atomic) 110 + udelay(QED_BAR_ACQUIRE_TIMEOUT_UDELAY); 111 + else 112 + usleep_range(QED_BAR_ACQUIRE_TIMEOUT_USLEEP, 113 + QED_BAR_ACQUIRE_TIMEOUT_USLEEP * 2); 122 114 } 123 115 124 116 DP_NOTICE(p_hwfn, "PTT acquire timeout - failed to allocate PTT\n");
+14 -5
drivers/net/ethernet/qlogic/qed/qed_iscsi.c
··· 999 999 } 1000 1000 1001 1001 static int qed_iscsi_get_stats(struct qed_hwfn *p_hwfn, 1002 - struct qed_iscsi_stats *stats) 1002 + struct qed_iscsi_stats *stats, 1003 + bool is_atomic) 1003 1004 { 1004 1005 struct qed_ptt *p_ptt; 1005 1006 1006 1007 memset(stats, 0, sizeof(*stats)); 1007 1008 1008 - p_ptt = qed_ptt_acquire(p_hwfn); 1009 + p_ptt = qed_ptt_acquire_context(p_hwfn, is_atomic); 1009 1010 if (!p_ptt) { 1010 1011 DP_ERR(p_hwfn, "Failed to acquire ptt\n"); 1011 1012 return -EAGAIN; ··· 1337 1336 QED_SPQ_MODE_EBLOCK, NULL); 1338 1337 } 1339 1338 1339 + static int qed_iscsi_stats_context(struct qed_dev *cdev, 1340 + struct qed_iscsi_stats *stats, 1341 + bool is_atomic) 1342 + { 1343 + return qed_iscsi_get_stats(QED_AFFIN_HWFN(cdev), stats, is_atomic); 1344 + } 1345 + 1340 1346 static int qed_iscsi_stats(struct qed_dev *cdev, struct qed_iscsi_stats *stats) 1341 1347 { 1342 - return qed_iscsi_get_stats(QED_AFFIN_HWFN(cdev), stats); 1348 + return qed_iscsi_stats_context(cdev, stats, false); 1343 1349 } 1344 1350 1345 1351 static int qed_iscsi_change_mac(struct qed_dev *cdev, ··· 1366 1358 } 1367 1359 1368 1360 void qed_get_protocol_stats_iscsi(struct qed_dev *cdev, 1369 - struct qed_mcp_iscsi_stats *stats) 1361 + struct qed_mcp_iscsi_stats *stats, 1362 + bool is_atomic) 1370 1363 { 1371 1364 struct qed_iscsi_stats proto_stats; 1372 1365 1373 1366 /* Retrieve FW statistics */ 1374 1367 memset(&proto_stats, 0, sizeof(proto_stats)); 1375 - if (qed_iscsi_stats(cdev, &proto_stats)) { 1368 + if (qed_iscsi_stats_context(cdev, &proto_stats, is_atomic)) { 1376 1369 DP_VERBOSE(cdev, QED_MSG_STORAGE, 1377 1370 "Failed to collect ISCSI statistics\n"); 1378 1371 return;
+6 -2
drivers/net/ethernet/qlogic/qed/qed_iscsi.h
··· 39 39 * 40 40 * @cdev: Qed dev pointer. 41 41 * @stats: Points to struct that will be filled with statistics. 42 + * @is_atomic: Hint from the caller - if the func can sleep or not. 42 43 * 44 + * Context: The function should not sleep in case is_atomic == true. 43 45 * Return: Void. 44 46 */ 45 47 void qed_get_protocol_stats_iscsi(struct qed_dev *cdev, 46 - struct qed_mcp_iscsi_stats *stats); 48 + struct qed_mcp_iscsi_stats *stats, 49 + bool is_atomic); 47 50 #else /* IS_ENABLED(CONFIG_QED_ISCSI) */ 48 51 static inline int qed_iscsi_alloc(struct qed_hwfn *p_hwfn) 49 52 { ··· 59 56 60 57 static inline void 61 58 qed_get_protocol_stats_iscsi(struct qed_dev *cdev, 62 - struct qed_mcp_iscsi_stats *stats) {} 59 + struct qed_mcp_iscsi_stats *stats, 60 + bool is_atomic) {} 63 61 #endif /* IS_ENABLED(CONFIG_QED_ISCSI) */ 64 62 65 63 #endif
+14 -5
drivers/net/ethernet/qlogic/qed/qed_l2.c
··· 1863 1863 } 1864 1864 1865 1865 static void _qed_get_vport_stats(struct qed_dev *cdev, 1866 - struct qed_eth_stats *stats) 1866 + struct qed_eth_stats *stats, 1867 + bool is_atomic) 1867 1868 { 1868 1869 u8 fw_vport = 0; 1869 1870 int i; ··· 1873 1872 1874 1873 for_each_hwfn(cdev, i) { 1875 1874 struct qed_hwfn *p_hwfn = &cdev->hwfns[i]; 1876 - struct qed_ptt *p_ptt = IS_PF(cdev) ? qed_ptt_acquire(p_hwfn) 1877 - : NULL; 1875 + struct qed_ptt *p_ptt; 1878 1876 bool b_get_port_stats; 1879 1877 1878 + p_ptt = IS_PF(cdev) ? qed_ptt_acquire_context(p_hwfn, is_atomic) 1879 + : NULL; 1880 1880 if (IS_PF(cdev)) { 1881 1881 /* The main vport index is relative first */ 1882 1882 if (qed_fw_vport(p_hwfn, 0, &fw_vport)) { ··· 1903 1901 1904 1902 void qed_get_vport_stats(struct qed_dev *cdev, struct qed_eth_stats *stats) 1905 1903 { 1904 + qed_get_vport_stats_context(cdev, stats, false); 1905 + } 1906 + 1907 + void qed_get_vport_stats_context(struct qed_dev *cdev, 1908 + struct qed_eth_stats *stats, 1909 + bool is_atomic) 1910 + { 1906 1911 u32 i; 1907 1912 1908 1913 if (!cdev || cdev->recov_in_prog) { ··· 1917 1908 return; 1918 1909 } 1919 1910 1920 - _qed_get_vport_stats(cdev, stats); 1911 + _qed_get_vport_stats(cdev, stats, is_atomic); 1921 1912 1922 1913 if (!cdev->reset_stats) 1923 1914 return; ··· 1969 1960 if (!cdev->reset_stats) { 1970 1961 DP_INFO(cdev, "Reset stats not allocated\n"); 1971 1962 } else { 1972 - _qed_get_vport_stats(cdev, cdev->reset_stats); 1963 + _qed_get_vport_stats(cdev, cdev->reset_stats, false); 1973 1964 cdev->reset_stats->common.link_change_count = 0; 1974 1965 } 1975 1966 }
+24
drivers/net/ethernet/qlogic/qed/qed_l2.h
··· 249 249 enum spq_mode comp_mode, 250 250 struct qed_spq_comp_cb *p_comp_data); 251 251 252 + /** 253 + * qed_get_vport_stats(): Fills provided statistics 254 + * struct with statistics. 255 + * 256 + * @cdev: Qed dev pointer. 257 + * @stats: Points to struct that will be filled with statistics. 258 + * 259 + * Return: Void. 260 + */ 252 261 void qed_get_vport_stats(struct qed_dev *cdev, struct qed_eth_stats *stats); 262 + 263 + /** 264 + * qed_get_vport_stats_context(): Fills provided statistics 265 + * struct with statistics. 266 + * 267 + * @cdev: Qed dev pointer. 268 + * @stats: Points to struct that will be filled with statistics. 269 + * @is_atomic: Hint from the caller - if the func can sleep or not. 270 + * 271 + * Context: The function should not sleep in case is_atomic == true. 272 + * Return: Void. 273 + */ 274 + void qed_get_vport_stats_context(struct qed_dev *cdev, 275 + struct qed_eth_stats *stats, 276 + bool is_atomic); 253 277 254 278 void qed_reset_vport_stats(struct qed_dev *cdev); 255 279
+3 -3
drivers/net/ethernet/qlogic/qed/qed_main.c
··· 3092 3092 3093 3093 switch (type) { 3094 3094 case QED_MCP_LAN_STATS: 3095 - qed_get_vport_stats(cdev, &eth_stats); 3095 + qed_get_vport_stats_context(cdev, &eth_stats, true); 3096 3096 stats->lan_stats.ucast_rx_pkts = 3097 3097 eth_stats.common.rx_ucast_pkts; 3098 3098 stats->lan_stats.ucast_tx_pkts = ··· 3100 3100 stats->lan_stats.fcs_err = -1; 3101 3101 break; 3102 3102 case QED_MCP_FCOE_STATS: 3103 - qed_get_protocol_stats_fcoe(cdev, &stats->fcoe_stats); 3103 + qed_get_protocol_stats_fcoe(cdev, &stats->fcoe_stats, true); 3104 3104 break; 3105 3105 case QED_MCP_ISCSI_STATS: 3106 - qed_get_protocol_stats_iscsi(cdev, &stats->iscsi_stats); 3106 + qed_get_protocol_stats_iscsi(cdev, &stats->iscsi_stats, true); 3107 3107 break; 3108 3108 default: 3109 3109 DP_VERBOSE(cdev, QED_MSG_SP,