| Message ID | 20250225145359.1126786-1-arnd@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] iwlegacy: don't warn for unused variables with DEBUG_FS=n | expand |
On Tue, Feb 25, 2025 at 03:53:53PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > The reference to il_rate_mcs is inside of an #ifdef, causing a W=1 warning: > > drivers/net/wireless/intel/iwlegacy/4965-rs.c:189:38: error: unused variable 'il_rate_mcs' [-Werror,-Wunused-const-variable] > static const struct il_rate_mcs_info il_rate_mcs[RATE_COUNT] = { > > Replace the #ifdef with a PTR_IF() for better compile time analysis. > The dead code will still get eliminated, but the warning goes away. But then the code will be compiled for !CONFIG_MAC80211_DEBUGFS case, it does compile for me: - 22475 1160 0 23635 5c53 drivers/net/wireless/intel/iwlegacy/4965-rs.o + 23008 1168 0 24176 5e70 drivers/net/wireless/intel/iwlegacy/4965-rs.o How about moving static const struct il_rate_mcs_info il_rate_mcs[RATE_COUNT] under CONFIG_MAC80211_DEBUGFS ? Maybe inside the function that use it ? Regards Stanislaw > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > v2: use correct config symbol consistently > --- > drivers/net/wireless/intel/iwlegacy/4965-rs.c | 15 ++------------- > drivers/net/wireless/intel/iwlegacy/common.h | 2 -- > 2 files changed, 2 insertions(+), 15 deletions(-) > > diff --git a/drivers/net/wireless/intel/iwlegacy/4965-rs.c b/drivers/net/wireless/intel/iwlegacy/4965-rs.c > index 718efb1aa1b0..f754fb979546 100644 > --- a/drivers/net/wireless/intel/iwlegacy/4965-rs.c > +++ b/drivers/net/wireless/intel/iwlegacy/4965-rs.c > @@ -132,15 +132,8 @@ static void il4965_rs_fill_link_cmd(struct il_priv *il, > static void il4965_rs_stay_in_table(struct il_lq_sta *lq_sta, > bool force_search); > > -#ifdef CONFIG_MAC80211_DEBUGFS > static void il4965_rs_dbgfs_set_mcs(struct il_lq_sta *lq_sta, > u32 *rate_n_flags, int idx); > -#else > -static void > -il4965_rs_dbgfs_set_mcs(struct il_lq_sta *lq_sta, u32 * rate_n_flags, int idx) > -{ > -} > -#endif > > /* > * The following tables contain the expected throughput metrics for all rates > @@ -2495,8 +2488,6 @@ il4965_rs_free_sta(void *il_r, struct ieee80211_sta *sta, void *il_sta) > D_RATE("leave\n"); > } > > -#ifdef CONFIG_MAC80211_DEBUGFS > - > static void > il4965_rs_dbgfs_set_mcs(struct il_lq_sta *lq_sta, u32 * rate_n_flags, int idx) > { > @@ -2758,7 +2749,6 @@ il4965_rs_add_debugfs(void *il, void *il_sta, struct dentry *dir) > debugfs_create_u8("tx_agg_tid_enable", 0600, dir, > &lq_sta->tx_agg_tid_en); > } > -#endif > > /* > * Initialization of rate scaling information is done by driver after > @@ -2781,9 +2771,8 @@ static const struct rate_control_ops rs_4965_ops = { > .free = il4965_rs_free, > .alloc_sta = il4965_rs_alloc_sta, > .free_sta = il4965_rs_free_sta, > -#ifdef CONFIG_MAC80211_DEBUGFS > - .add_sta_debugfs = il4965_rs_add_debugfs, > -#endif > + .add_sta_debugfs = PTR_IF(IS_ENABLED(CONFIG_MAC80211_DEBUGFS), > + il4965_rs_add_debugfs), > }; > > int > diff --git a/drivers/net/wireless/intel/iwlegacy/common.h b/drivers/net/wireless/intel/iwlegacy/common.h > index 92285412ab10..52610f5e57a3 100644 > --- a/drivers/net/wireless/intel/iwlegacy/common.h > +++ b/drivers/net/wireless/intel/iwlegacy/common.h > @@ -2815,9 +2815,7 @@ struct il_lq_sta { > struct il_scale_tbl_info lq_info[LQ_SIZE]; /* "active", "search" */ > struct il_traffic_load load[TID_MAX_LOAD_COUNT]; > u8 tx_agg_tid_en; > -#ifdef CONFIG_MAC80211_DEBUGFS > u32 dbg_fixed_rate; > -#endif > struct il_priv *drv; > > /* used to be in sta_info */ > -- > 2.39.5 >
On Sat, Mar 01, 2025 at 01:38:16PM +0100, Arnd Bergmann wrote: > On Sat, Mar 1, 2025, at 13:28, Stanislaw Gruszka wrote: > > On Tue, Feb 25, 2025 at 03:53:53PM +0100, Arnd Bergmann wrote: > > > > But then the code will be compiled for !CONFIG_MAC80211_DEBUGFS > > case, it does compile for me: > > > > - 22475 1160 0 23635 > > 5c53 drivers/net/wireless/intel/iwlegacy/4965-rs.o > > + 23008 1168 0 24176 > > 5e70 drivers/net/wireless/intel/iwlegacy/4965-rs.o > > Very strange, this really shouldn't happen. Which symbols > exactly do you see the compiler fail to drop with my patch, nm 4965-rs.o diffrence before and after patch: U ieee80211_rate_control_register U ieee80211_rate_control_unregister U ieee80211_start_tx_ba_session U ieee80211_stop_tx_ba_session 0000000000000010 t il4965_hwrate_to_plcp_idx -00000000000043b0 T il4965_rate_control_register -00000000000043e0 T il4965_rate_control_unregister +00000000000043f0 T il4965_rate_control_register +0000000000004420 T il4965_rate_control_unregister 00000000000002e0 t il4965_rate_n_flags_from_tbl 0000000000000000 t il4965_rate_n_flags_from_tbl.cold 0000000000000270 t il4965_rs_alloc 0000000000000710 t il4965_rs_alloc_sta 00000000000000dd t il4965_rs_alloc_sta.cold -0000000000001810 t il4965_rs_collect_tx_data.isra.0 -00000000000012b0 t il4965_rs_fill_link_cmd -0000000000000495 t il4965_rs_fill_link_cmd.cold +0000000000001850 t il4965_rs_collect_tx_data.isra.0 +0000000000000e90 t il4965_rs_dbgfs_set_mcs.isra.0 +00000000000002f6 t il4965_rs_dbgfs_set_mcs.isra.0.cold +0000000000001340 t il4965_rs_fill_link_cmd +0000000000000518 t il4965_rs_fill_link_cmd.cold 00000000000002a0 t il4965_rs_free 0000000000000e40 t il4965_rs_free_sta 000000000000028d t il4965_rs_free_sta.cold @@ -173,31 +184,31 @@ 0000000000000a20 t il4965_rs_get_best_rate 0000000000000bb0 t il4965_rs_get_rate 0000000000000209 t il4965_rs_get_rate.cold -0000000000001180 t il4965_rs_get_tbl_info_from_mcs.isra.0 -0000000000003bd0 T il4965_rs_rate_init -0000000000000d5f t il4965_rs_rate_init.cold +0000000000001210 t il4965_rs_get_tbl_info_from_mcs.isra.0 +0000000000003c10 T il4965_rs_rate_init +0000000000000de4 t il4965_rs_rate_init.cold 00000000000002c0 t il4965_rs_rate_init_stub 0000000000000760 t il4965_rs_set_expected_tpt_table 0000000000000490 t il4965_rs_stay_in_table 0000000000000088 t il4965_rs_stay_in_table.cold -0000000000001000 t il4965_rs_switch_to_mimo2.isra.0 -00000000000003c3 t il4965_rs_switch_to_mimo2.isra.0.cold -0000000000000e90 t il4965_rs_switch_to_siso.isra.0 -00000000000002f6 t il4965_rs_switch_to_siso.isra.0.cold +0000000000001090 t il4965_rs_switch_to_mimo2.isra.0 +0000000000000446 t il4965_rs_switch_to_mimo2.isra.0.cold +0000000000000f20 t il4965_rs_switch_to_siso.isra.0 +0000000000000379 t il4965_rs_switch_to_siso.isra.0.cold 00000000000000d0 t il4965_rs_tl_rm_old_stats 00000000000001a0 t il4965_rs_toggle_antenna -0000000000001970 t il4965_rs_tx_status -00000000000004d4 t il4965_rs_tx_status.cold +00000000000019b0 t il4965_rs_tx_status +0000000000000559 t il4965_rs_tx_status.cold U il_debug_level U il_is_ht40_tx_allowed -0000000000001180 R il_rates +00000000000012a0 R il_rates U il_send_lq_cmd U jiffies U jiffies_to_msecs 000000000000004b r .LC15 0000000000000070 r .LC9 -0000000000001100 r rs_4965_ops -0000000000001210 r rs_ht_to_legacy +0000000000001220 r rs_4965_ops +0000000000001330 r rs_ht_to_legacy U __stack_chk_fail U __ubsan_handle_out_of_bounds U __ubsan_handle_shift_out_of_bounds > and which compiler version are you using? It is: gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2) I've checked on other system with gcc (GCC) 14.2.1 20240912 (Red Hat 14.2.1-3) and there size difference is similar: - 28876 4875 0 33751 83d7 drivers/net/wireless/intel/iwlegacy/4965-rs.o + 29454 4851 0 34305 8601 drivers/net/wireless/intel/iwlegacy/4965-rs.o > > How about moving > > static const struct il_rate_mcs_info il_rate_mcs[RATE_COUNT] > > under CONFIG_MAC80211_DEBUGFS ? Maybe inside the function that use it ? > > It's not supposed to make a difference, let's try to figure > out if there is a compiler bug or a mistake in my patch first > and then fix it in the right place. Regards Stanislaw
On Sat, Mar 01, 2025 at 02:49:38PM +0100, Arnd Bergmann wrote: > On Sat, Mar 1, 2025, at 14:36, Stanislaw Gruszka wrote: > > On Sat, Mar 01, 2025 at 01:38:16PM +0100, Arnd Bergmann wrote: > >> On Sat, Mar 1, 2025, at 13:28, Stanislaw Gruszka wrote: > >> > On Tue, Feb 25, 2025 at 03:53:53PM +0100, Arnd Bergmann wrote: > >> > > >> > But then the code will be compiled for !CONFIG_MAC80211_DEBUGFS > >> > case, it does compile for me: > >> > > >> > - 22475 1160 0 23635 > >> > 5c53 drivers/net/wireless/intel/iwlegacy/4965-rs.o > >> > + 23008 1168 0 24176 > >> > 5e70 drivers/net/wireless/intel/iwlegacy/4965-rs.o > >> > >> Very strange, this really shouldn't happen. Which symbols > >> exactly do you see the compiler fail to drop with my patch, > > > > nm 4965-rs.o diffrence before and after patch: > > > 00000000000000dd t il4965_rs_alloc_sta.cold > > -0000000000001810 t il4965_rs_collect_tx_data.isra.0 > > -00000000000012b0 t il4965_rs_fill_link_cmd > > -0000000000000495 t il4965_rs_fill_link_cmd.cold > > +0000000000001850 t il4965_rs_collect_tx_data.isra.0 > > +0000000000000e90 t il4965_rs_dbgfs_set_mcs.isra.0 > > +00000000000002f6 t il4965_rs_dbgfs_set_mcs.isra.0.cold > > +0000000000001340 t il4965_rs_fill_link_cmd > > +0000000000000518 t il4965_rs_fill_link_cmd.cold > > 00000000000002a0 t il4965_rs_free > > Ah, so the debugfs files get eliminated, but > il4965_rs_dbgfs_set_mcs() does not. > > I think this should do it: > > --- a/drivers/net/wireless/intel/iwlegacy/4965-rs.c > +++ b/drivers/net/wireless/intel/iwlegacy/4965-rs.c > @@ -2495,6 +2495,9 @@ il4965_rs_dbgfs_set_mcs(struct il_lq_sta *lq_sta, u32 * rate_n_flags, int idx) > u8 valid_tx_ant; > u8 ant_sel_tx; > > + if (!IS_ENABLED(CONFIG_MAC80211_DEBUGFS)) > + return; > + > il = lq_sta->drv; > valid_tx_ant = il->hw_params.valid_tx_ant; > if (lq_sta->dbg_fixed_rate) { > > or possibly il4965_rs_dbgfs_set_mcs() can stay in the #ifdef > if you prefer. I'm ok with this solution. The size stays the same with above change. Regards Stanislaw
diff --git a/drivers/net/wireless/intel/iwlegacy/4965-rs.c b/drivers/net/wireless/intel/iwlegacy/4965-rs.c index 718efb1aa1b0..f754fb979546 100644 --- a/drivers/net/wireless/intel/iwlegacy/4965-rs.c +++ b/drivers/net/wireless/intel/iwlegacy/4965-rs.c @@ -132,15 +132,8 @@ static void il4965_rs_fill_link_cmd(struct il_priv *il, static void il4965_rs_stay_in_table(struct il_lq_sta *lq_sta, bool force_search); -#ifdef CONFIG_MAC80211_DEBUGFS static void il4965_rs_dbgfs_set_mcs(struct il_lq_sta *lq_sta, u32 *rate_n_flags, int idx); -#else -static void -il4965_rs_dbgfs_set_mcs(struct il_lq_sta *lq_sta, u32 * rate_n_flags, int idx) -{ -} -#endif /* * The following tables contain the expected throughput metrics for all rates @@ -2495,8 +2488,6 @@ il4965_rs_free_sta(void *il_r, struct ieee80211_sta *sta, void *il_sta) D_RATE("leave\n"); } -#ifdef CONFIG_MAC80211_DEBUGFS - static void il4965_rs_dbgfs_set_mcs(struct il_lq_sta *lq_sta, u32 * rate_n_flags, int idx) { @@ -2758,7 +2749,6 @@ il4965_rs_add_debugfs(void *il, void *il_sta, struct dentry *dir) debugfs_create_u8("tx_agg_tid_enable", 0600, dir, &lq_sta->tx_agg_tid_en); } -#endif /* * Initialization of rate scaling information is done by driver after @@ -2781,9 +2771,8 @@ static const struct rate_control_ops rs_4965_ops = { .free = il4965_rs_free, .alloc_sta = il4965_rs_alloc_sta, .free_sta = il4965_rs_free_sta, -#ifdef CONFIG_MAC80211_DEBUGFS - .add_sta_debugfs = il4965_rs_add_debugfs, -#endif + .add_sta_debugfs = PTR_IF(IS_ENABLED(CONFIG_MAC80211_DEBUGFS), + il4965_rs_add_debugfs), }; int diff --git a/drivers/net/wireless/intel/iwlegacy/common.h b/drivers/net/wireless/intel/iwlegacy/common.h index 92285412ab10..52610f5e57a3 100644 --- a/drivers/net/wireless/intel/iwlegacy/common.h +++ b/drivers/net/wireless/intel/iwlegacy/common.h @@ -2815,9 +2815,7 @@ struct il_lq_sta { struct il_scale_tbl_info lq_info[LQ_SIZE]; /* "active", "search" */ struct il_traffic_load load[TID_MAX_LOAD_COUNT]; u8 tx_agg_tid_en; -#ifdef CONFIG_MAC80211_DEBUGFS u32 dbg_fixed_rate; -#endif struct il_priv *drv; /* used to be in sta_info */