> -----Original Message----- > From: Nguyen, Anthony L <[email protected]> > Sent: Thursday, February 22, 2024 2:10 PM > To: Brandeburg, Jesse <[email protected]>; intel-wired- > [email protected] > Cc: [email protected]; Elliott, Rob <[email protected]>; Keller, Jacob E > <[email protected]>; David S. Miller <[email protected]>; Eric > Dumazet > <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni > <[email protected]>; Michal Swiatkowski <[email protected]> > Subject: Re: [PATCH iwl-net v1] ice: fix NULL pointer access during resume > > On 2/20/2024 3:17 PM, Jesse Brandeburg wrote: > > The ice_suspend/ice_resume cycle was not updated when refactoring was > > done to the init path and I suspect this allowed a bug to creep in where > > the driver was not correctly reinitialized during resume. > > > > I was able to test against 6.1.77 kernel and that ice driver works fine > > for suspend/resume with no panic. > > > > Instead of tearing down interrupts and freeing a bunch of memory during > > suspend, just begin an internal reset event, which takes care of all the > > correct steps during suspend. Likewise during resume we'll just let the > > reset complete and the driver comes right back to life. This mirrors the > > behavior of other suspend/resume code in drivers like fm10k. > > > > Older kernel commits were made to this driver and to the i40e driver to > > try to fix "disk" or hibernate suspend events with many CPUs. The PM > > subsystem was updated since then but the drivers kept the old flows. > > Testing with rtcwake -m [disk | mem] -s 10 - passes but my system won't > > hibernate due to too much RAM, not enough swap. > > > > The code is slightly refactored during this change in order to share a > > common "prep" path between suspend and the pci error handler functions > > which all do the same thing, so introduce ice_quiesce_before_reset(). > > > > While doing all this and compile testing I ran across the pm.h changes > > to get rid of compilation problems when CONFIG_PM=n etc, so those small > > changes are included here as well. > > > > ... > > > > > Fixes: 5b246e533d01 ("ice: split probe into smaller functions") > > Reported-by: Robert Elliott <[email protected]> > > Reviewed-by: Jacob Keller <[email protected]> > > Signed-off-by: Jesse Brandeburg <[email protected]> > > --- > > NOTE: > > Requires Amritha's patch: > > https://patchwork.ozlabs.org/project/intel-wired- > lan/patch/[email protected]. > intel.com/ > > to be applied before this will pass testing cleanly. > > I think this may be the other way around? It looks to be clean for > netdev (doesn't have Amritha's patch), but it's not applying to > net-queue (has Amritha's patch). > > > base-commit: 23f9c2c066e7e5052406fb8f04a115d3d0260b22 > > Base commit also seems to be a netdev commit. > > Since Amritha's patch is pending to netdev [1], I think we need a > version that will apply with Amritha's changes. > > Thanks, > Tony > > > [1] > https://lore.kernel.org/netdev/20240220214444.1039759-7- > [email protected]/
I think he means that the tests won't pass without Amritha's patch because we'll hit the ASSERT_RTNL in the suspend flow otherwise. Thanks, Jake
