From: Benjamin Poirier <bpoirier@suse.com>
To: Ben Hutchings <ben.hutchings@codethink.co.uk>
Cc: Gabriel C <nix.or.die@gmail.com>, Christian Hesse <list@eworm.de>,
Jeff Kirsher <jeffrey.t.kirsher@intel.com>,
stable@vger.kernel.org,
Lennart Sorensen <lsorense@csclub.uwaterloo.ca>,
Aaron Brown <aaron.f.brown@intel.com>,
Amit Pundir <amit.pundir@linaro.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4.4 71/96] e1000e: Separate signaling for link check/link up
Date: Fri, 8 Dec 2017 17:34:41 +0900 [thread overview]
Message-ID: <20171208083441.kvyilu3hhf5pae3q@f1.synalogic.ca> (raw)
In-Reply-To: <1512676979.18523.193.camel@codethink.co.uk>
On 2017/12/07 20:02, Ben Hutchings wrote:
> On Tue, 2017-11-28 at 11:23 +0100, Greg Kroah-Hartman wrote:
> > 4.4-stable review patch. If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Benjamin Poirier <bpoirier@suse.com>
> >
> > commit 19110cfbb34d4af0cdfe14cd243f3b09dc95b013 upstream.
> [...]
> > --- a/drivers/net/ethernet/intel/e1000e/mac.c
> > +++ b/drivers/net/ethernet/intel/e1000e/mac.c
> > @@ -410,6 +410,9 @@ void e1000e_clear_hw_cntrs_base(struct e
> > * Checks to see of the link status of the hardware has changed. If a
> > * change in link status has been detected, then we read the PHY registers
> > * to get the current speed/duplex if link exists.
> > + *
> > + * Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link
> > + * up).
> > **/
> > s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
> > {
> [...]
> > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> > @@ -5017,7 +5017,7 @@ static bool e1000e_has_link(struct e1000
> > > case e1000_media_type_copper:
> > > if (hw->mac.get_link_status) {
> > > ret_val = hw->mac.ops.check_for_link(hw);
> > > - link_active = !hw->mac.get_link_status;
> > > + link_active = ret_val > 0;
> > > } else {
> > > link_active = true;
> > > }
>
> As this change in e1000e_has_link() is conditional only on the media
> type, doesn't e1000_check_for_copper_link_ich8lan() also need to be
> changed to return 1 for link up?
You're right. I looked at it again, in the commit log I wrote that
"hw->mac.ops.check_for_link(hw) === e1000e_check_for_copper_link" which
is true for the race condition reported (because that's the function in
use on adapters that have msix vectors mac.type == e1000_82574) but not
generally true. The other check_for_link callback needs to be adjusted
likewise.
However, I happen to have a I218-LM (e1000_pch_lpt) so I tested 4.14.3
and this error only delays link up, it doesn't prevent it.
e1000_check_for_copper_link_ich8lan() sets mac->get_link_status = false;
and on the next watchdog execution, we fall in the second branch of the
following e1000e_has_link code:
case e1000_media_type_copper:
if (hw->mac.get_link_status) {
ret_val = hw->mac.ops.check_for_link(hw);
link_active = ret_val > 0;
} else {
link_active = true;
OTOH, there are multiple reports in
https://bugzilla.kernel.org/show_bug.cgi?id=198047
that reverting 830466993daf ("e1000e: Separate signaling for link
check/link up") fixes the issue so there's something I'm missing.
Gabriel and Christian, can you test the following patch?
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index d6d4ed7acf03..31277d3bb7dc 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -1367,6 +1367,9 @@ static s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force)
* Checks to see of the link status of the hardware has changed. If a
* change in link status has been detected, then we read the PHY registers
* to get the current speed/duplex if link exists.
+ *
+ * Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link
+ * up).
**/
static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
{
@@ -1382,7 +1385,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
* Change or Rx Sequence Error interrupt.
*/
if (!mac->get_link_status)
- return 0;
+ return 1;
/* First we want to see if the MII Status Register reports
* link. If so, then we want to get the current speed/duplex
@@ -1613,10 +1616,12 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
* different link partner.
*/
ret_val = e1000e_config_fc_after_link_up(hw);
- if (ret_val)
+ if (ret_val) {
e_dbg("Error configuring flow control\n");
+ return ret_val;
+ }
- return ret_val;
+ return 1;
}
static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter)
--
2.15.1
next prev parent reply other threads:[~2017-12-08 8:34 UTC|newest]
Thread overview: 125+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-28 10:22 [PATCH 4.4 00/96] 4.4.103-stable review Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 02/96] s390/runtime instrumention: fix possible memory corruption Greg Kroah-Hartman
2017-12-05 17:02 ` Ben Hutchings
2017-12-05 17:08 ` Greg Kroah-Hartman
2017-12-05 18:15 ` Heiko Carstens
2017-12-06 7:44 ` Greg Kroah-Hartman
2017-12-06 13:30 ` Heiko Carstens
2017-12-06 13:53 ` Greg Kroah-Hartman
2017-12-06 13:31 ` Heiko Carstens
2017-11-28 10:22 ` [PATCH 4.4 03/96] s390/disassembler: add missing end marker for e7 table Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 04/96] s390/disassembler: increase show_code buffer size Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 05/96] ipv6: only call ip6_route_dev_notify() once for NETDEV_UNREGISTER Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 06/96] AF_VSOCK: Shrink the area influenced by prepare_to_wait Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 07/96] vsock: use new wait API for vsock_stream_sendmsg() Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 08/96] sched: Make resched_cpu() unconditional Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 09/96] lib/mpi: call cond_resched() from mpi_powm() loop Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 10/96] x86/decoder: Add new TEST instruction pattern Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 11/96] ARM: 8722/1: mm: make STRICT_KERNEL_RWX effective for LPAE Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 12/96] ARM: 8721/1: mm: dump: check hardware RO bit " Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 13/96] MIPS: ralink: Fix MT7628 pinmux Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 14/96] MIPS: ralink: Fix typo in mt7628 pinmux function Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 15/96] ALSA: hda: Add Raven PCI ID Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 16/96] dm bufio: fix integer overflow when limiting maximum cache size Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 17/96] dm: fix race between dm_get_from_kobject() and __dm_destroy() Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 18/96] MIPS: Fix an n32 core file generation regset support regression Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 20/96] autofs: dont fail mount for transient error Greg Kroah-Hartman
2017-12-05 21:59 ` Ben Hutchings
2017-12-05 22:22 ` NeilBrown
2017-11-28 10:22 ` [PATCH 4.4 21/96] nilfs2: fix race condition that causes file system corruption Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 22/96] eCryptfs: use after free in ecryptfs_release_messaging() Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 23/96] bcache: check ca->alloc_thread initialized before wake up it Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 24/96] isofs: fix timestamps beyond 2027 Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 25/96] NFS: Fix typo in nomigration mount option Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 26/96] nfs: Fix ugly referral attributes Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 27/96] nfsd: deal with revoked delegations appropriately Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 28/96] rtlwifi: rtl8192ee: Fix memory leak when loading firmware Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 29/96] rtlwifi: fix uninitialized rtlhal->last_suspend_sec time Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 30/96] ata: fixes kernel crash while tracing ata_eh_link_autopsy event Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 31/96] ext4: fix interaction between i_size, fallocate, and delalloc after a crash Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 32/96] ALSA: pcm: update tstamp only if audio_tstamp changed Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 33/96] ALSA: usb-audio: Add sanity checks to FE parser Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 34/96] ALSA: usb-audio: Fix potential out-of-bound access at parsing SU Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 35/96] ALSA: usb-audio: Add sanity checks in v2 clock parsers Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 36/96] ALSA: timer: Remove kernel warning at compat ioctl error paths Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 37/96] ALSA: hda/realtek - Fix ALC700 family no sound issue Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 38/96] fix a page leak in vhost_scsi_iov_to_sgl() error recovery Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 39/96] fs/9p: Compare qid.path in v9fs_test_inode Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 40/96] iscsi-target: Fix non-immediate TMR reference leak Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 41/96] target: Fix QUEUE_FULL + SCSI task attribute handling Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 42/96] KVM: nVMX: set IDTR and GDTR limits when loading L1 host state Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 44/96] SUNRPC: Fix tracepoint storage issues with svc_recv and svc_rqst_status Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 45/96] clk: ti: dra7-atl-clock: Fix of_node reference counting Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 46/96] clk: ti: dra7-atl-clock: fix child-node lookups Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 47/96] libnvdimm, namespace: fix label initialization to use valid seq numbers Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 48/96] libnvdimm, namespace: make resource attribute only readable by root Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 49/96] IB/srpt: Do not accept invalid initiator port names Greg Kroah-Hartman
2017-11-28 10:22 ` [PATCH 4.4 50/96] IB/srp: Avoid that a cable pull can trigger a kernel crash Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 51/96] NFC: fix device-allocation error return Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 52/96] i40e: Use smp_rmb rather than read_barrier_depends Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 53/96] igb: " Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 54/96] igbvf: " Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 55/96] ixgbevf: " Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 56/96] i40evf: " Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 57/96] fm10k: " Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 58/96] ixgbe: Fix skb list corruption on Power systems Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 59/96] parisc: Fix validity check of pointer size argument in new CAS implementation Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 60/96] powerpc/signal: Properly handle return value from uprobe_deny_signal() Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 61/96] media: Dont do DMA on stack for firmware upload in the AS102 driver Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 62/96] media: rc: check for integer overflow Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 63/96] [media] cx231xx-cards: fix NULL-deref on missing association descriptor Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 64/96] media: v4l2-ctrl: Fix flags field on Control events Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 65/96] sched/rt: Simplify the IPI based RT balancing logic Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 66/96] fscrypt: lock mutex before checking for bounce page pool Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 67/96] net/9p: Switch to wait_event_killable() Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 68/96] PM / OPP: Add missing of_node_put(np) Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 69/96] e1000e: Fix error path in link detection Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 70/96] e1000e: Fix return value test Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 71/96] e1000e: Separate signaling for link check/link up Greg Kroah-Hartman
2017-12-07 20:02 ` Ben Hutchings
2017-12-08 8:34 ` Benjamin Poirier [this message]
2017-12-08 10:45 ` Christian Hesse
2017-12-11 7:26 ` [PATCH] e1000e: Fix e1000_check_for_copper_link_ich8lan return value Benjamin Poirier
2017-12-21 6:57 ` [Intel-wired-lan] " Neftin, Sasha
2017-12-22 22:00 ` Brown, Aaron F
2017-11-28 10:23 ` [PATCH 4.4 72/96] RDS: RDMA: return appropriate error on rdma map failures Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 73/96] PCI: Apply _HPX settings only to relevant devices Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 74/96] dmaengine: zx: set DMA_CYCLIC cap_mask bit Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 75/96] net: Allow IP_MULTICAST_IF to set index to L3 slave Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 76/96] net: 3com: typhoon: typhoon_init_one: make return values more specific Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 77/96] net: 3com: typhoon: typhoon_init_one: fix incorrect return values Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 78/96] drm/armada: Fix compile fail Greg Kroah-Hartman
2017-12-07 20:16 ` Ben Hutchings
2017-12-09 5:39 ` alexander.levin
2017-11-28 10:23 ` [PATCH 4.4 79/96] ath10k: fix incorrect txpower set by P2P_DEVICE interface Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 80/96] ath10k: ignore configuring the incorrect board_id Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 81/96] ath10k: fix potential memory leak in ath10k_wmi_tlv_op_pull_fw_stats() Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 82/96] ath10k: set CTS protection VDEV param only if VDEV is up Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 83/96] ALSA: hda - Apply ALC269_FIXUP_NO_SHUTUP on HDA_FIXUP_ACT_PROBE Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 84/96] drm: Apply range restriction after color adjustment when allocation Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 85/96] mac80211: Remove invalid flag operations in mesh TSF synchronization Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 86/96] mac80211: Suppress NEW_PEER_CANDIDATE event if no room Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 87/96] iio: light: fix improper return value Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 88/96] staging: iio: cdc: " Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 89/96] spi: SPI_FSL_DSPI should depend on HAS_DMA Greg Kroah-Hartman
2017-12-07 20:41 ` Ben Hutchings
2017-12-09 5:40 ` alexander.levin
2017-11-28 10:23 ` [PATCH 4.4 90/96] netfilter: nft_queue: use raw_smp_processor_id() Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 91/96] netfilter: nf_tables: fix oob access Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 92/96] ASoC: rsnd: dont double free kctrl Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 93/96] btrfs: return the actual error value from from btrfs_uuid_tree_iterate Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 94/96] ASoC: wm_adsp: Dont overrun firmware file buffer when reading region data Greg Kroah-Hartman
2017-11-28 10:23 ` [PATCH 4.4 95/96] s390/kbuild: enable modversions for symbols exported from asm Greg Kroah-Hartman
2017-12-07 21:21 ` Ben Hutchings
2017-12-09 5:40 ` alexander.levin
2017-11-28 10:23 ` [PATCH 4.4 96/96] xen: xenbus driver must not accept invalid transaction ids Greg Kroah-Hartman
2017-11-28 17:26 ` [PATCH 4.4 00/96] 4.4.103-stable review Naresh Kamboju
2017-11-29 8:07 ` Greg Kroah-Hartman
2017-11-29 9:53 ` Naresh Kamboju
2017-11-29 10:36 ` Greg Kroah-Hartman
2017-11-29 13:51 ` Naresh Kamboju
2017-11-28 18:27 ` Nathan Chancellor
2017-11-29 8:07 ` Greg Kroah-Hartman
2017-11-28 19:46 ` Shuah Khan
2017-11-28 21:51 ` Guenter Roeck
2017-12-09 21:00 [PATCH 4.4 71/96] e1000e: Separate signaling for link check/link up rwarsow
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171208083441.kvyilu3hhf5pae3q@f1.synalogic.ca \
--to=bpoirier@suse.com \
--cc=aaron.f.brown@intel.com \
--cc=amit.pundir@linaro.org \
--cc=ben.hutchings@codethink.co.uk \
--cc=gregkh@linuxfoundation.org \
--cc=jeffrey.t.kirsher@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=list@eworm.de \
--cc=lsorense@csclub.uwaterloo.ca \
--cc=nix.or.die@gmail.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).