Messages in this thread | | | Subject | Re: [Intel-wired-lan] [PATCH] igc: Report speed and duplex as unknown when device is runtime suspended | From | "Neftin, Sasha" <> | Date | Thu, 3 Dec 2020 11:12:32 +0200 |
| |
On 12/2/2020 09:50, Kai-Heng Feng wrote: > Similar to commit 165ae7a8feb5 ("igb: Report speed and duplex as unknown > when device is runtime suspended"), if we try to read speed and duplex > sysfs while the device is runtime suspeneded, igc will complain and > stops working: > > [ 123.449883] igc 0000:03:00.0 enp3s0: PCIe link lost, device now detached > [ 123.450052] BUG: kernel NULL pointer dereference, address: 0000000000000008 > [ 123.450056] #PF: supervisor read access in kernel mode > [ 123.450058] #PF: error_code(0x0000) - not-present page > [ 123.450059] PGD 0 P4D 0 > [ 123.450064] Oops: 0000 [#1] SMP NOPTI > [ 123.450068] CPU: 0 PID: 2525 Comm: udevadm Tainted: G U W OE 5.10.0-1002-oem #2+rkl2-Ubuntu > [ 123.450078] RIP: 0010:igc_rd32+0x1c/0x90 [igc] > [ 123.450080] Code: c0 5d c3 b8 fd ff ff ff c3 0f 1f 44 00 00 0f 1f 44 00 00 55 89 f0 48 89 e5 41 56 41 55 41 54 49 89 c4 53 48 8b 57 08 48 01 d0 <44> 8b 28 41 83 fd ff 74 0c 5b 44 89 e8 41 5c 41 5d 4 > > [ 123.450083] RSP: 0018:ffffb0d100d6fcc0 EFLAGS: 00010202 > [ 123.450085] RAX: 0000000000000008 RBX: ffffb0d100d6fd30 RCX: 0000000000000000 > [ 123.450087] RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff945a12716c10 > [ 123.450089] RBP: ffffb0d100d6fce0 R08: ffff945a12716550 R09: ffff945a09874000 > [ 123.450090] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000008 > [ 123.450092] R13: ffff945a12716000 R14: ffff945a037da280 R15: ffff945a037da290 > [ 123.450094] FS: 00007f3b34c868c0(0000) GS:ffff945b89200000(0000) knlGS:0000000000000000 > [ 123.450096] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 123.450098] CR2: 0000000000000008 CR3: 00000001144de006 CR4: 0000000000770ef0 > [ 123.450100] PKRU: 55555554 > [ 123.450101] Call Trace: > [ 123.450111] igc_ethtool_get_link_ksettings+0xd6/0x1b0 [igc] > [ 123.450118] __ethtool_get_link_ksettings+0x71/0xb0 > [ 123.450123] duplex_show+0x74/0xc0 > [ 123.450129] dev_attr_show+0x1d/0x40 > [ 123.450134] sysfs_kf_seq_show+0xa1/0x100 > [ 123.450137] kernfs_seq_show+0x27/0x30 > [ 123.450142] seq_read+0xb7/0x400 > [ 123.450148] ? common_file_perm+0x72/0x170 > [ 123.450151] kernfs_fop_read+0x35/0x1b0 > [ 123.450155] vfs_read+0xb5/0x1b0 > [ 123.450157] ksys_read+0x67/0xe0 > [ 123.450160] __x64_sys_read+0x1a/0x20 > [ 123.450164] do_syscall_64+0x38/0x90 > [ 123.450168] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [ 123.450170] RIP: 0033:0x7f3b351fe142 > [ 123.450173] Code: c0 e9 c2 fe ff ff 50 48 8d 3d 3a ca 0a 00 e8 f5 19 02 00 0f 1f 44 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 0f 05 <48> 3d 00 f0 ff ff 77 56 c3 0f 1f 44 00 00 48 83 ec 28 48 89 54 24 > [ 123.450174] RSP: 002b:00007fffef2ec138 EFLAGS: 00000246 ORIG_RAX: 0000000000000000 > [ 123.450177] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f3b351fe142 > [ 123.450179] RDX: 0000000000001001 RSI: 00005644c047f070 RDI: 0000000000000003 > [ 123.450180] RBP: 00007fffef2ec340 R08: 00005644c047f070 R09: 00007f3b352d9320 > [ 123.450182] R10: 00005644c047c010 R11: 0000000000000246 R12: 00005644c047cbf0 > [ 123.450184] R13: 00005644c047e6d0 R14: 0000000000000003 R15: 00007fffef2ec140 > [ 123.450189] Modules linked in: rfcomm ccm cmac algif_hash algif_skcipher af_alg bnep toshiba_acpi industrialio toshiba_haps hp_accel lis3lv02d btusb btrtl btbcm btintel bluetooth ecdh_generic ecc joydev input_leds nls_iso8859_1 snd_sof_pci snd_sof_intel_byt snd_sof_intel_ipc snd_sof_intel_hda_common snd_soc_hdac_hda snd_hda_codec_hdmi snd_sof_xtensa_dsp snd_sof_intel_hda snd_sof snd_hda_ext_core snd_soc_acpi_intel_match snd_soc_acpi snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_intel snd_intel_dspcfg soundwire_intel soundwire_generic_allocation soundwire_cadence snd_hda_codec snd_hda_core ath10k_pci snd_hwdep intel_rapl_msr intel_rapl_common ath10k_core soundwire_bus snd_soc_core x86_pkg_temp_thermal ath intel_powerclamp snd_compress ac97_bus snd_pcm_dmaengine mac80211 snd_pcm coretemp snd_seq_midi snd_seq_midi_event snd_rawmidi kvm_intel cfg80211 snd_seq snd_seq_device snd_timer mei_hdcp kvm libarc4 snd crct10dif_pclmul ghash_clmulni_intel aesni_intel > mei_me dell_wmi > [ 123.450266] dell_smbios soundcore sparse_keymap dcdbas crypto_simd cryptd mei dell_uart_backlight glue_helper ee1004 wmi_bmof intel_wmi_thunderbolt dell_wmi_descriptor mac_hid efi_pstore acpi_pad acpi_tad intel_cstate sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid1 raid0 multipath linear dm_mirror dm_region_hash dm_log hid_generic usbhid hid i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec crc32_pclmul rc_core drm intel_lpss_pci i2c_i801 ahci igc intel_lpss i2c_smbus idma64 xhci_pci libahci virt_dma xhci_pci_renesas wmi video pinctrl_tigerlake > [ 123.450335] CR2: 0000000000000008 > [ 123.450338] ---[ end trace 9f731e38b53c35cc ]--- > > The more generic approach will be wrap get_link_ksettings() with begin() > and complete() callbacks, and calls runtime resume and runtime suspend > routine respectively. However, igc is like igb, runtime resume routine > uses rtnl_lock() which upper ethtool layer also uses. > > So to prevent a deadlock on rtnl, take a different approach, use > pm_runtime_suspended() to avoid reading register while device is runtime > suspended. > > Cc: <stable@vger.kernel.org> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/net/ethernet/intel/igc/igc_ethtool.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c > index 61d331ce38cd..4b9eac9dc090 100644 > --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c > +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c > @@ -1708,7 +1708,8 @@ static int igc_ethtool_get_link_ksettings(struct net_device *netdev, > Asym_Pause); > } > > - status = rd32(IGC_STATUS); > + status = pm_runtime_suspended(&adapter->pdev->dev) ? > + 0 : rd32(IGC_STATUS); > > if (status & IGC_STATUS_LU) { > if (status & IGC_STATUS_SPEED_1000) { > Acked-by: Sasha Neftin <sasha.neftin@intel.com>
| |