Re: [PATCH net] ice: fix memory leak of aRFS after resuming from suspend
From: Creeley, Brett <hidden>
Date: 2021-03-18 22:21:07
On Thu, 2021-03-18 at 16:15 +0800, Yongxin Liu wrote:
In ice_suspend(), ice_clear_interrupt_scheme() is called, and then irq_free_descs() will be eventually called to free irq and its descriptor. In ice_resume(), ice_init_interrupt_scheme() is called to allocate new irqs. However, in ice_rebuild_arfs(), struct irq_glue and struct cpu_rmap maybe cannot be freed, if the irqs that released in ice_suspend() were reassigned to other devices, which makes irq descriptor's affinity_notify lost. So move ice_remove_arfs() before ice_clear_interrupt_scheme(), which can make sure all irq_glue and cpu_rmap can be correctly released before corresponding irq and descriptor are released. Fix the following memeory leak.
s/memeory/memory <snip>
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/intel/ice/ice_arfs.cb/drivers/net/ethernet/intel/ice/ice_arfs.c index 6560acd76c94..c748d0a5c7d4 100644--- a/drivers/net/ethernet/intel/ice/ice_arfs.c +++ b/drivers/net/ethernet/intel/ice/ice_arfs.c@@ -654,7 +654,6 @@ void ice_rebuild_arfs(struct ice_pf *pf) if (!pf_vsi) return; - ice_remove_arfs(pf);
This should not be removed. Removing this would break the reset flows outside of the suspend/remove case.
quoted hunk ↗ jump to hunk
if (ice_set_cpu_rx_rmap(pf_vsi)) { dev_err(ice_pf_to_dev(pf), "Failed to rebuild aRFS\n"); return;diff --git a/drivers/net/ethernet/intel/ice/ice_main.cb/drivers/net/ethernet/intel/ice/ice_main.c index 2c23c8f468a5..dba901bf2b9b 100644--- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c@@ -4568,6 +4568,9 @@ static int __maybe_unused ice_suspend(structdevice *dev) continue; ice_vsi_free_q_vectors(pf->vsi[v]); } + if (test_bit(ICE_FLAG_FD_ENA, pf->flags)) { + ice_remove_arfs(pf); + }
Braces aren't needed around a single if statement like this. Also, I don't think this is the right solution. I think a better approach would be to call ice_free_rx_cpu_map() here. With this, it seems like no other changes are necessary. It also isn't necessary to check the ICE_FLAG_FD_ENA bit with this change.
ice_clear_interrupt_scheme(pf); pci_save_state(pdev);