| Message ID | 20240222145025.722515-3-maciej.fijalkowski@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | intel: misc improvements | expand |
On 2/22/24 15:50, Maciej Fijalkowski wrote: > 1. pcaps are free'd right after AQ routines are done, no need for > devm_'s > 2. a test frame for loopback test in ethtool -t is destroyed at the end > of the test so we don't need devm_ here either. > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_common.c | 23 +++++++++----------- > drivers/net/ethernet/intel/ice/ice_ethtool.c | 4 ++-- > 2 files changed, 12 insertions(+), 15 deletions(-) > nice, thank you! BTW, we are committed to using Scope Based Resource Management [1] even in the OOT driver, so you could consider that for future IWL submissions :) [1] https://lwn.net/Articles/934679/ I'm also happy with this as-is too, so: Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Hi Maciej, kernel test robot noticed the following build warnings: [auto build test WARNING on v6.8-rc5] [also build test WARNING on linus/master next-20240223] [cannot apply to tnguy-next-queue/dev-queue tnguy-net-queue/dev-queue horms-ipvs/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Maciej-Fijalkowski/ice-do-not-disable-Tx-queues-twice-in-ice_down/20240222-225134 base: v6.8-rc5 patch link: https://lore.kernel.org/r/20240222145025.722515-3-maciej.fijalkowski%40intel.com patch subject: [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240223/202402231718.8mWcBppj-lkp@intel.com/config) compiler: alpha-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240223/202402231718.8mWcBppj-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202402231718.8mWcBppj-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/net/ethernet/intel/ice/ice_common.c: In function 'ice_update_link_info': >> drivers/net/ethernet/intel/ice/ice_common.c:3242:32: warning: variable 'hw' set but not used [-Wunused-but-set-variable] 3242 | struct ice_hw *hw; | ^~ -- drivers/net/ethernet/intel/ice/ice_ethtool.c: In function 'ice_loopback_test': >> drivers/net/ethernet/intel/ice/ice_ethtool.c:947:24: warning: variable 'dev' set but not used [-Wunused-but-set-variable] 947 | struct device *dev; | ^~~ vim +/hw +3242 drivers/net/ethernet/intel/ice/ice_common.c fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3221 fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3222 /** fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3223 * ice_update_link_info - update status of the HW network link fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3224 * @pi: port info structure of the interested logical port fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3225 */ 5e24d5984c805c Tony Nguyen 2021-10-07 3226 int ice_update_link_info(struct ice_port_info *pi) fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3227 { 092a33d4031205 Bruce Allan 2019-04-16 3228 struct ice_link_status *li; 5e24d5984c805c Tony Nguyen 2021-10-07 3229 int status; fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3230 fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3231 if (!pi) d54699e27d506f Tony Nguyen 2021-10-07 3232 return -EINVAL; fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3233 092a33d4031205 Bruce Allan 2019-04-16 3234 li = &pi->phy.link_info; fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3235 fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3236 status = ice_aq_get_link_info(pi, true, NULL, NULL); fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3237 if (status) 092a33d4031205 Bruce Allan 2019-04-16 3238 return status; 092a33d4031205 Bruce Allan 2019-04-16 3239 092a33d4031205 Bruce Allan 2019-04-16 3240 if (li->link_info & ICE_AQ_MEDIA_AVAILABLE) { 092a33d4031205 Bruce Allan 2019-04-16 3241 struct ice_aqc_get_phy_caps_data *pcaps; 092a33d4031205 Bruce Allan 2019-04-16 @3242 struct ice_hw *hw; 092a33d4031205 Bruce Allan 2019-04-16 3243 092a33d4031205 Bruce Allan 2019-04-16 3244 hw = pi->hw; f8543c3af0dcb2 Maciej Fijalkowski 2024-02-22 3245 pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL); 092a33d4031205 Bruce Allan 2019-04-16 3246 if (!pcaps) d54699e27d506f Tony Nguyen 2021-10-07 3247 return -ENOMEM; fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3248 d6730a871e68f1 Anirudh Venkataramanan 2021-03-25 3249 status = ice_aq_get_phy_caps(pi, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA, fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3250 pcaps, NULL); fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3251 f8543c3af0dcb2 Maciej Fijalkowski 2024-02-22 3252 kfree(pcaps); 092a33d4031205 Bruce Allan 2019-04-16 3253 } 092a33d4031205 Bruce Allan 2019-04-16 3254 fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3255 return status; fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3256 } fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20 3257
On Fri, Feb 23, 2024 at 08:45:20AM +0100, Przemek Kitszel wrote: > On 2/22/24 15:50, Maciej Fijalkowski wrote: > > 1. pcaps are free'd right after AQ routines are done, no need for > > devm_'s > > 2. a test frame for loopback test in ethtool -t is destroyed at the end > > of the test so we don't need devm_ here either. > > > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > --- > > drivers/net/ethernet/intel/ice/ice_common.c | 23 +++++++++----------- > > drivers/net/ethernet/intel/ice/ice_ethtool.c | 4 ++-- > > 2 files changed, 12 insertions(+), 15 deletions(-) > > > > nice, thank you! > > BTW, we are committed to using Scope Based Resource Management [1] even > in the OOT driver, so you could consider that for future IWL submissions > :) Thanks for reminding me about this, will do that. > > [1] https://lwn.net/Articles/934679/ > > > I'm also happy with this as-is too, so: > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c index 10c32cd80fff..6de93e12ead3 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.c +++ b/drivers/net/ethernet/intel/ice/ice_common.c @@ -1045,7 +1045,7 @@ int ice_init_hw(struct ice_hw *hw) if (status) goto err_unroll_sched; - pcaps = devm_kzalloc(ice_hw_to_dev(hw), sizeof(*pcaps), GFP_KERNEL); + pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL); if (!pcaps) { status = -ENOMEM; goto err_unroll_sched; @@ -1055,7 +1055,7 @@ int ice_init_hw(struct ice_hw *hw) status = ice_aq_get_phy_caps(hw->port_info, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA, pcaps, NULL); - devm_kfree(ice_hw_to_dev(hw), pcaps); + kfree(pcaps); if (status) dev_warn(ice_hw_to_dev(hw), "Get PHY capabilities failed status = %d, continuing anyway\n", status); @@ -1082,18 +1082,16 @@ int ice_init_hw(struct ice_hw *hw) /* Get MAC information */ /* A single port can report up to two (LAN and WoL) addresses */ - mac_buf = devm_kcalloc(ice_hw_to_dev(hw), 2, - sizeof(struct ice_aqc_manage_mac_read_resp), - GFP_KERNEL); - mac_buf_len = 2 * sizeof(struct ice_aqc_manage_mac_read_resp); - + mac_buf = kcalloc(2, sizeof(struct ice_aqc_manage_mac_read_resp), + GFP_KERNEL); if (!mac_buf) { status = -ENOMEM; goto err_unroll_fltr_mgmt_struct; } + mac_buf_len = 2 * sizeof(struct ice_aqc_manage_mac_read_resp); status = ice_aq_manage_mac_read(hw, mac_buf, mac_buf_len, NULL); - devm_kfree(ice_hw_to_dev(hw), mac_buf); + kfree(mac_buf); if (status) goto err_unroll_fltr_mgmt_struct; @@ -3244,15 +3242,14 @@ int ice_update_link_info(struct ice_port_info *pi) struct ice_hw *hw; hw = pi->hw; - pcaps = devm_kzalloc(ice_hw_to_dev(hw), sizeof(*pcaps), - GFP_KERNEL); + pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL); if (!pcaps) return -ENOMEM; status = ice_aq_get_phy_caps(pi, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA, pcaps, NULL); - devm_kfree(ice_hw_to_dev(hw), pcaps); + kfree(pcaps); } return status; @@ -3404,7 +3401,7 @@ ice_set_fc(struct ice_port_info *pi, u8 *aq_failures, bool ena_auto_link_update) *aq_failures = 0; hw = pi->hw; - pcaps = devm_kzalloc(ice_hw_to_dev(hw), sizeof(*pcaps), GFP_KERNEL); + pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL); if (!pcaps) return -ENOMEM; @@ -3456,7 +3453,7 @@ ice_set_fc(struct ice_port_info *pi, u8 *aq_failures, bool ena_auto_link_update) } out: - devm_kfree(ice_hw_to_dev(hw), pcaps); + kfree(pcaps); return status; } diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c index a19b06f18e40..cec3d796546e 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c @@ -801,7 +801,7 @@ static int ice_lbtest_create_frame(struct ice_pf *pf, u8 **ret_data, u16 size) if (!pf) return -EINVAL; - data = devm_kzalloc(ice_pf_to_dev(pf), size, GFP_KERNEL); + data = kzalloc(size, GFP_KERNEL); if (!data) return -ENOMEM; @@ -1004,7 +1004,7 @@ static u64 ice_loopback_test(struct net_device *netdev) ret = 10; lbtest_free_frame: - devm_kfree(dev, tx_frame); + kfree(tx_frame); remove_mac_filters: if (ice_fltr_remove_mac(test_vsi, broadcast, ICE_FWD_TO_VSI)) netdev_err(netdev, "Could not remove MAC filter for the test VSI\n");
1. pcaps are free'd right after AQ routines are done, no need for devm_'s 2. a test frame for loopback test in ethtool -t is destroyed at the end of the test so we don't need devm_ here either. Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> --- drivers/net/ethernet/intel/ice/ice_common.c | 23 +++++++++----------- drivers/net/ethernet/intel/ice/ice_ethtool.c | 4 ++-- 2 files changed, 12 insertions(+), 15 deletions(-)