On Tuesday, 16 November 2021 05:15:21 CET Wen Gong wrote: > Currently ath11k has device recovery logic, it is introduced by this > patch "ath11k: Add support for subsystem recovery" which is upstream > by https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath11k-bringup&id=3a7b4838b6f6f234239f263ef3dc02e612a083ad. https://www.kernel.org/doc/html/v5.15/process/submitting-patches.html#describe-your-changes Please search for "If you want to refer to a specific commit" And this commit you referenced is definitely not the upstream commit. It was only part of Kalle's repository. The code was only upstreamed with commit d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices"). Btw. another thing which I see again and again in these patches: > --- a/drivers/net/wireless/ath/ath11k/core.h > +++ b/drivers/net/wireless/ath/ath11k/core.h > @@ -39,6 +39,10 @@ [...] > + bool is_reset; https://www.kernel.org/doc/html/v5.15/process/coding-style.html#using-bool "If a structure has many true/false values, consider consolidating them into a bitfield with 1 bit members, or using an appropriate fixed width type, such as u8." There are more verbose mails from Linus Torvalds where he points out the problems of bools in structs. See for example struct ath11k_skb_rxcb. At the moment, it looks like this: struct ath11k_skb_rxcb { dma_addr_t paddr; bool is_first_msdu; bool is_last_msdu; bool is_continuation; bool is_mcbc; bool is_eapol_tkip; struct hal_rx_desc *rx_desc; u8 err_rel_src; u8 err_code; u8 mac_id; u8 unmapped; u8 is_frag; u8 tid; u16 peer_id; u16 seq_no; struct napi_struct *napi; }; Compiled for IPQ60xx, it would end up as: $ pahole ./drivers/net/wireless/ath/ath11k/ath11k.o -C ath11k_skb_rxcb struct ath11k_skb_rxcb { dma_addr_t paddr; /* 0 8 */ bool is_first_msdu; /* 8 1 */ bool is_last_msdu; /* 9 1 */ bool is_continuation; /* 10 1 */ bool is_mcbc; /* 11 1 */ bool is_eapol_tkip; /* 12 1 */ /* XXX 3 bytes hole, try to pack */ struct hal_rx_desc * rx_desc; /* 16 8 */ u8 err_rel_src; /* 24 1 */ u8 err_code; /* 25 1 */ u8 mac_id; /* 26 1 */ u8 unmapped; /* 27 1 */ u8 is_frag; /* 28 1 */ u8 tid; /* 29 1 */ u16 peer_id; /* 30 2 */ u16 seq_no; /* 32 2 */ /* XXX 6 bytes hole, try to pack */ struct napi_struct * napi; /* 40 8 */ /* size: 48, cachelines: 1, members: 16 */ /* sum members: 39, holes: 2, sum holes: 9 */ /* last cacheline: 48 bytes */ }; After changing it to u8 ....:1 and reorganizing the structure: $ pahole drivers/net/wireless/ath/ath11k/ath11k.o -C ath11k_skb_rxcb -R struct ath11k_skb_rxcb { dma_addr_t paddr; /* 0 8 */ struct hal_rx_desc * rx_desc; /* 8 8 */ struct napi_struct * napi; /* 16 8 */ u8 err_rel_src; /* 24 1 */ u8 err_code; /* 25 1 */ u8 mac_id; /* 26 1 */ u8 unmapped; /* 27 1 */ u8 is_frag; /* 28 1 */ u8 tid; /* 29 1 */ u8 is_first_msdu:1; /* 30: 0 1 */ u8 is_last_msdu:1; /* 30: 1 1 */ u8 is_continuation:1; /* 30: 2 1 */ u8 is_mcbc:1; /* 30: 3 1 */ u8 is_eapol_tkip:1; /* 30: 4 1 */ /* XXX 3 bits hole, try to pack */ /* XXX 1 byte hole, try to pack */ u16 peer_id; /* 32 2 */ u16 seq_no; /* 34 2 */ /* size: 40, cachelines: 1, members: 16 */ /* sum members: 34, holes: 1, sum holes: 1 */ /* sum bitfield members: 5 bits, bit holes: 1, sum bit holes: 3 bits */ /* padding: 4 */ /* last cacheline: 40 bytes */ }; Or ath11k_bus_params: $ pahole ./drivers/net/wireless/ath/ath11k/ath11k.o -C ath11k_bus_params struct ath11k_bus_params { bool mhi_support; /* 0 1 */ bool m3_fw_support; /* 1 1 */ bool fixed_bdf_addr; /* 2 1 */ bool fixed_mem_region; /* 3 1 */ bool static_window_map; /* 4 1 */ /* size: 5, cachelines: 1, members: 5 */ /* last cacheline: 5 bytes */ }; After changing it to an "u8 ....:1": $ pahole ./drivers/net/wireless/ath/ath11k/ath11k.o -C ath11k_bus_params struct ath11k_bus_params { u8 mhi_support:1; /* 0: 0 1 */ u8 m3_fw_support:1; /* 0: 1 1 */ u8 fixed_bdf_addr:1; /* 0: 2 1 */ u8 fixed_mem_region:1; /* 0: 3 1 */ u8 static_window_map:1; /* 0: 4 1 */ /* size: 1, cachelines: 1, members: 5 */ /* bit_padding: 3 bits */ /* last cacheline: 1 bytes */ }; There are even better examples. ath11k_hw_params will for example take currently 200 bytes. With a little reordering and adjusting of bools, it can easily be reduced to 152 bytes. But other structures might have more impact because they are used more often. Kind regards, Sven
Attachment:
signature.asc
Description: This is a digitally signed message part.
