Hello Kang Yang,
Commit 3a415daa3e8b ("wifi: ath11k: add P2P IE in beacon template")
from Feb 28, 2024 (linux-next), leads to the following Smatch static
checker warning:
drivers/net/wireless/ath/ath11k/wmi.c:1742 ath11k_wmi_p2p_go_bcn_ie()
warn: sleeping in atomic context
drivers/net/wireless/ath/ath11k/wmi.c
1712 int ath11k_wmi_p2p_go_bcn_ie(struct ath11k *ar, u32 vdev_id,
1713 const u8 *p2p_ie)
1714 {
1715 struct ath11k_pdev_wmi *wmi = ar->wmi;
1716 struct wmi_p2p_go_set_beacon_ie_cmd *cmd;
1717 size_t p2p_ie_len, aligned_len;
1718 struct wmi_tlv *tlv;
1719 struct sk_buff *skb;
1720 int ret, len;
1721
1722 p2p_ie_len = p2p_ie[1] + 2;
1723 aligned_len = roundup(p2p_ie_len, 4);
1724
1725 len = sizeof(*cmd) + TLV_HDR_SIZE + aligned_len;
1726
1727 skb = ath11k_wmi_alloc_skb(wmi->wmi_ab, len);
1728 if (!skb)
1729 return -ENOMEM;
1730
1731 cmd = (struct wmi_p2p_go_set_beacon_ie_cmd *)skb->data;
1732 cmd->tlv_header = FIELD_PREP(WMI_TLV_TAG, WMI_TAG_P2P_GO_SET_BEACON_IE) |
1733 FIELD_PREP(WMI_TLV_LEN, sizeof(*cmd) - TLV_HDR_SIZE);
1734 cmd->vdev_id = vdev_id;
1735 cmd->ie_buf_len = p2p_ie_len;
1736
1737 tlv = (struct wmi_tlv *)cmd->tlv;
1738 tlv->header = FIELD_PREP(WMI_TLV_TAG, WMI_TAG_ARRAY_BYTE) |
1739 FIELD_PREP(WMI_TLV_LEN, aligned_len);
1740 memcpy(tlv->value, p2p_ie, p2p_ie_len);
1741
--> 1742 ret = ath11k_wmi_cmd_send(wmi, skb, WMI_P2P_GO_SET_BEACON_IE);
^^^^^^^^^^^^^^^^^^^
This is a might_sleep() function.
1743 if (ret) {
1744 ath11k_warn(ar->ab, "failed to send WMI_P2P_GO_SET_BEACON_IE\n");
1745 dev_kfree_skb(skb);
1746 }
1747
1748 return ret;
1749 }
The problematic call tree is:
ath11k_bcn_tx_status_event() <- disables preempt
-> ath11k_mac_bcn_tx_event()
-> ath11k_mac_setup_bcn_tmpl()
-> ath11k_mac_setup_bcn_tmpl_ema()
-> ath11k_mac_setup_bcn_tmpl_mbssid()
-> ath11k_mac_set_vif_params()
-> ath11k_mac_setup_bcn_p2p_ie()
-> ath11k_wmi_p2p_go_bcn_ie()
The ath11k_bcn_tx_status_event() function takes rcu_read_lock() which
disables preemption. I don't know the code well enough to say if this
is a real bug... If it's a false positive, just ignore it. These are
one time emails.
See my blog for more details.
https://staticthinking.wordpress.com/2024/05/24/sleeping-in-atomic-warnings/
regards,
dan carpenter
On 5/29/2024 10:32 PM, Dan Carpenter wrote:
> Hello Kang Yang,
>
> Commit 3a415daa3e8b ("wifi: ath11k: add P2P IE in beacon template")
> from Feb 28, 2024 (linux-next), leads to the following Smatch static
> checker warning:
>
> drivers/net/wireless/ath/ath11k/wmi.c:1742 ath11k_wmi_p2p_go_bcn_ie()
> warn: sleeping in atomic context
>
> drivers/net/wireless/ath/ath11k/wmi.c
> 1712 int ath11k_wmi_p2p_go_bcn_ie(struct ath11k *ar, u32 vdev_id,
> 1713 const u8 *p2p_ie)
> 1714 {
> 1715 struct ath11k_pdev_wmi *wmi = ar->wmi;
> 1716 struct wmi_p2p_go_set_beacon_ie_cmd *cmd;
> 1717 size_t p2p_ie_len, aligned_len;
> 1718 struct wmi_tlv *tlv;
> 1719 struct sk_buff *skb;
> 1720 int ret, len;
> 1721
> 1722 p2p_ie_len = p2p_ie[1] + 2;
> 1723 aligned_len = roundup(p2p_ie_len, 4);
> 1724
> 1725 len = sizeof(*cmd) + TLV_HDR_SIZE + aligned_len;
> 1726
> 1727 skb = ath11k_wmi_alloc_skb(wmi->wmi_ab, len);
> 1728 if (!skb)
> 1729 return -ENOMEM;
> 1730
> 1731 cmd = (struct wmi_p2p_go_set_beacon_ie_cmd *)skb->data;
> 1732 cmd->tlv_header = FIELD_PREP(WMI_TLV_TAG, WMI_TAG_P2P_GO_SET_BEACON_IE) |
> 1733 FIELD_PREP(WMI_TLV_LEN, sizeof(*cmd) - TLV_HDR_SIZE);
> 1734 cmd->vdev_id = vdev_id;
> 1735 cmd->ie_buf_len = p2p_ie_len;
> 1736
> 1737 tlv = (struct wmi_tlv *)cmd->tlv;
> 1738 tlv->header = FIELD_PREP(WMI_TLV_TAG, WMI_TAG_ARRAY_BYTE) |
> 1739 FIELD_PREP(WMI_TLV_LEN, aligned_len);
> 1740 memcpy(tlv->value, p2p_ie, p2p_ie_len);
> 1741
> --> 1742 ret = ath11k_wmi_cmd_send(wmi, skb, WMI_P2P_GO_SET_BEACON_IE);
> ^^^^^^^^^^^^^^^^^^^
> This is a might_sleep() function.
>
> 1743 if (ret) {
> 1744 ath11k_warn(ar->ab, "failed to send WMI_P2P_GO_SET_BEACON_IE\n");
> 1745 dev_kfree_skb(skb);
> 1746 }
> 1747
> 1748 return ret;
> 1749 }
>
> The problematic call tree is:
>
> ath11k_bcn_tx_status_event() <- disables preempt
> -> ath11k_mac_bcn_tx_event()
> -> ath11k_mac_setup_bcn_tmpl()
> -> ath11k_mac_setup_bcn_tmpl_ema()
> -> ath11k_mac_setup_bcn_tmpl_mbssid()
> -> ath11k_mac_set_vif_params()
> -> ath11k_mac_setup_bcn_p2p_ie()
> -> ath11k_wmi_p2p_go_bcn_ie()
>
> The ath11k_bcn_tx_status_event() function takes rcu_read_lock() which
> disables preemption. I don't know the code well enough to say if this
> is a real bug... If it's a false positive, just ignore it. These are
> one time emails.
I also found:
ath11k_bcn_tx_status_event() <- disables preempt
-> ath11k_mac_bcn_tx_event()
-> ath11k_mac_setup_bcn_tmpl()
-> ath11k_mac_setup_bcn_tmpl_ema()
-> ath11k_mac_setup_bcn_tmpl_mbssid()
->ath11k_wmi_bcn_tmpl()
->ath11k_wmi_cmd_send()
It seems this problem already exist even if without my patch.
Fine, i will find solution for this.
>
> See my blog for more details.
> https://staticthinking.wordpress.com/2024/05/24/sleeping-in-atomic-warnings/
> > regards,
> dan carpenter