Re: [PULL] topic/e1000e-fix

Re: [PULL] topic/e1000e-fix

[Daniel Vetter]

On Wed, May 31, 2017 at 7:54 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Wed, May 31, 2017 at 1:06 AM, Dave Airlie <airlied@gmail.com> wrote:
>> On 31 May 2017 at 08:10, David Miller <davem@davemloft.net> wrote:
>>> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Date: Tue, 30 May 2017 22:15:42 +0200
>>>
>>>> If the e1000e maintainer wants to coalesce or not return statements
>>>> this simple way, that's imo on him to change the color as needed.
>>>
>>> That's not how things work.
>>>
>>> If the maintainer wants you to style things a certain way, either you
>>> do it that way or your patch isn't accepted.
>
> Consider this pull a regression report, pls handle it.

And I guess I pile of more cc, to make this regression report
complete. I mean you got the backtrace, bisect and a proposed fix, and
the almost-whitespace change demanded is something gcc does in its
sleep. I'd understand a request to retest if it would be a real
functional change, but in this situation I have no idea why this
regression just can't be fixed already.

Not sure if it's really preferred if regression reports come
incomplete, without bugfix and bisect attached.
-Daniel

>> I'm not really sure why Chris just couldn't respin already.
>>
>> Though really I think Chris should have just asked for a revert of the
>> original patch that broke stuff, instead of trying to patch a driver
>> if he doesn't have time to get the patch right for the maintainer.
>
> Ok, can we pls revert 2800209994f8 ("e1000e: Refactor PM flows") then?
>
> Thanks, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PULL] topic/e1000e-fix

[David Miller]

From: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed, 31 May 2017 08:10:45 +0200

> On Wed, May 31, 2017 at 7:54 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> On Wed, May 31, 2017 at 1:06 AM, Dave Airlie <airlied@gmail.com> wrote:
>>> On 31 May 2017 at 08:10, David Miller <davem@davemloft.net> wrote:
>>>> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Date: Tue, 30 May 2017 22:15:42 +0200
>>>>
>>>>> If the e1000e maintainer wants to coalesce or not return statements
>>>>> this simple way, that's imo on him to change the color as needed.
>>>>
>>>> That's not how things work.
>>>>
>>>> If the maintainer wants you to style things a certain way, either you
>>>> do it that way or your patch isn't accepted.
>>
>> Consider this pull a regression report, pls handle it.
> 
> And I guess I pile of more cc, to make this regression report
> complete. I mean you got the backtrace, bisect and a proposed fix, and
> the almost-whitespace change demanded is something gcc does in its
> sleep. I'd understand a request to retest if it would be a real
> functional change, but in this situation I have no idea why this
> regression just can't be fixed already.

And we can't understand why respinning with the requested change is
less work than making several postings such as this one.

Re: [PULL] topic/e1000e-fix

[Jani Nikula]

On Wed, 31 May 2017, David Miller <davem@davemloft.net> wrote:
> And we can't understand why respinning with the requested change is
> less work than making several postings such as this one.

When our CI hits tons of non-drm issues every merge window, I imagine
our developers can start to get a little frustrated trying to get them
fixed. Especially so when valid fixes like this get blocked by silly
bikesheds from developers who don't even seem to have very many
contributions upstream, and the maintainers don't respond. We have
enough on our plates trying to get our own stuff fixed.

While I understand why our developers aren't willing to jump through all
the hoops, updated patch follows. Please let's move on.

BR,
Jani.


Chris Wilson (1):
  e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

 drivers/net/ethernet/intel/e1000e/netdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

[Jani Nikula]

From: Chris Wilson <chris@chris-wilson.co.uk>

An error during suspend (e100e_pm_suspend),

[  429.994338] ACPI : EC: event blocked
[  429.994633] e1000e: EEE TX LPI TIMER: 00000011
[  430.955451] pci_pm_suspend(): e1000e_pm_suspend+0x0/0x30 [e1000e] returns -2
[  430.955454] dpm_run_callback(): pci_pm_suspend+0x0/0x140 returns -2
[  430.955458] PM: Device 0000:00:19.0 failed to suspend async: error -2
[  430.955581] PM: Some devices failed to suspend, or early wake event detected
[  430.957709] ACPI : EC: event unblocked

lead to complete failure:

[  432.585002] ------------[ cut here ]------------
[  432.585013] WARNING: CPU: 3 PID: 8372 at kernel/irq/manage.c:1478 __free_irq+0x9f/0x280
[  432.585015] Trying to free already-free IRQ 20
[  432.585016] Modules linked in: cdc_ncm usbnet x86_pkg_temp_thermal intel_powerclamp coretemp mii crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep lpc_ich snd_hda_core snd_pcm mei_me mei sdhci_pci sdhci i915 mmc_core e1000e ptp pps_core prime_numbers
[  432.585042] CPU: 3 PID: 8372 Comm: kworker/u16:40 Tainted: G     U          4.10.0-rc8-CI-Patchwork_3870+ #1
[  432.585044] Hardware name: LENOVO 2356GCG/2356GCG, BIOS G7ET31WW (1.13 ) 07/02/2012
[  432.585050] Workqueue: events_unbound async_run_entry_fn
[  432.585051] Call Trace:
[  432.585058]  dump_stack+0x67/0x92
[  432.585062]  __warn+0xc6/0xe0
[  432.585065]  warn_slowpath_fmt+0x4a/0x50
[  432.585070]  ? _raw_spin_lock_irqsave+0x49/0x60
[  432.585072]  __free_irq+0x9f/0x280
[  432.585075]  free_irq+0x34/0x80
[  432.585089]  e1000_free_irq+0x65/0x70 [e1000e]
[  432.585098]  e1000e_pm_freeze+0x7a/0xb0 [e1000e]
[  432.585106]  e1000e_pm_suspend+0x21/0x30 [e1000e]
[  432.585113]  pci_pm_suspend+0x71/0x140
[  432.585118]  dpm_run_callback+0x6f/0x330
[  432.585122]  ? pci_pm_freeze+0xe0/0xe0
[  432.585125]  __device_suspend+0xea/0x330
[  432.585128]  async_suspend+0x1a/0x90
[  432.585132]  async_run_entry_fn+0x34/0x160
[  432.585137]  process_one_work+0x1f4/0x6d0
[  432.585140]  ? process_one_work+0x16e/0x6d0
[  432.585143]  worker_thread+0x49/0x4a0
[  432.585145]  kthread+0x107/0x140
[  432.585148]  ? process_one_work+0x6d0/0x6d0
[  432.585150]  ? kthread_create_on_node+0x40/0x40
[  432.585154]  ret_from_fork+0x2e/0x40
[  432.585156] ---[ end trace 6712df7f8c4b9124 ]---

The unwind failures stems from commit 2800209994f8 ("e1000e: Refactor PM
flows"), but it may be a later patch that introduced the non-recoverable
behaviour.

Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Dave Ertman <davidx.m.ertman@intel.com>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
[Jani: bikeshed repainted]
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index b3679728caac..5cad688be609 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6630,12 +6630,17 @@ static int e1000e_pm_thaw(struct device *dev)
 static int e1000e_pm_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
+	int rc;
 
 	e1000e_flush_lpic(pdev);
 
 	e1000e_pm_freeze(dev);
 
-	return __e1000_shutdown(pdev, false);
+	rc = __e1000_shutdown(pdev, false);
+	if (rc)
+		e1000e_pm_thaw(dev);
+
+	return rc;
 }
 
 static int e1000e_pm_resume(struct device *dev)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

[David Miller]

From: Jani Nikula <jani.nikula@intel.com>
Date: Wed, 31 May 2017 18:50:43 +0300

> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> An error during suspend (e100e_pm_suspend),
 ...
> lead to complete failure:
 ...
> The unwind failures stems from commit 2800209994f8 ("e1000e: Refactor PM
> flows"), but it may be a later patch that introduced the non-recoverable
> behaviour.
> 
> Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Dave Ertman <davidx.m.ertman@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> [Jani: bikeshed repainted]
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Jeff, please make sure this gets submitted to me soon.

Thanks.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]

On Fri, 2017-06-02 at 14:14 -0400, David Miller wrote:
> From: Jani Nikula <jani.nikula@intel.com>
> Date: Wed, 31 May 2017 18:50:43 +0300
> 
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > An error during suspend (e100e_pm_suspend),
> 
>  ...
> > lead to complete failure:
> 
>  ...
> > The unwind failures stems from commit 2800209994f8 ("e1000e:
> > Refactor PM
> > flows"), but it may be a later patch that introduced the non-
> > recoverable
> > behaviour.
> > 
> > Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Cc: Dave Ertman <davidx.m.ertman@intel.com>
> > Cc: Bruce Allan <bruce.w.allan@intel.com>
> > Cc: intel-wired-lan@lists.osuosl.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > [Jani: bikeshed repainted]
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> Jeff, please make sure this gets submitted to me soon.

Expect it later tonight, just finishing up testing.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

[Brown, Aaron F]

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Jeff Kirsher
> Sent: Tuesday, June 6, 2017 1:46 PM
> To: David Miller <davem@davemloft.net>; Nikula, Jani
> <jani.nikula@intel.com>
> Cc: Ursulin, Tvrtko <tvrtko.ursulin@intel.com>; daniel.vetter@ffwll.ch; intel-
> gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> jani.nikula@linux.intel.com; chris@chris-wilson.co.uk; Ertman, David M
> <david.m.ertman@intel.com>; intel-wired-lan@lists.osuosl.org; dri-
> devel@lists.freedesktop.org; netdev@vger.kernel.org; airlied@gmail.com
> Subject: Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo
> e1000e_pm_freeze if __e1000_shutdown fails
> 
> On Fri, 2017-06-02 at 14:14 -0400, David Miller wrote:
> > From: Jani Nikula <jani.nikula@intel.com>
> > Date: Wed, 31 May 2017 18:50:43 +0300
> >
> > > From: Chris Wilson <chris@chris-wilson.co.uk>
> > >
> > > An error during suspend (e100e_pm_suspend),
> >
> >  ...
> > > lead to complete failure:
> >
> >  ...
> > > The unwind failures stems from commit 2800209994f8 ("e1000e:
> > > Refactor PM
> > > flows"), but it may be a later patch that introduced the non-
> > > recoverable
> > > behaviour.
> > >
> > > Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > Cc: Dave Ertman <davidx.m.ertman@intel.com>
> > > Cc: Bruce Allan <bruce.w.allan@intel.com>
> > > Cc: intel-wired-lan@lists.osuosl.org
> > > Cc: netdev@vger.kernel.org
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > [Jani: bikeshed repainted]
> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >
> > Jeff, please make sure this gets submitted to me soon.
> 
> Expect it later tonight, just finishing up testing.

Tested-by: Aaron Brown <aaron.f.brown@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

[Daniel Vetter]

On Wed, Jun 07, 2017 at 01:07:33AM +0000, Brown, Aaron F wrote:
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> > Of Jeff Kirsher
> > Sent: Tuesday, June 6, 2017 1:46 PM
> > To: David Miller <davem@davemloft.net>; Nikula, Jani
> > <jani.nikula@intel.com>
> > Cc: Ursulin, Tvrtko <tvrtko.ursulin@intel.com>; daniel.vetter@ffwll.ch; intel-
> > gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> > jani.nikula@linux.intel.com; chris@chris-wilson.co.uk; Ertman, David M
> > <david.m.ertman@intel.com>; intel-wired-lan@lists.osuosl.org; dri-
> > devel@lists.freedesktop.org; netdev@vger.kernel.org; airlied@gmail.com
> > Subject: Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo
> > e1000e_pm_freeze if __e1000_shutdown fails
> > 
> > On Fri, 2017-06-02 at 14:14 -0400, David Miller wrote:
> > > From: Jani Nikula <jani.nikula@intel.com>
> > > Date: Wed, 31 May 2017 18:50:43 +0300
> > >
> > > > From: Chris Wilson <chris@chris-wilson.co.uk>
> > > >
> > > > An error during suspend (e100e_pm_suspend),
> > >
> > >  ...
> > > > lead to complete failure:
> > >
> > >  ...
> > > > The unwind failures stems from commit 2800209994f8 ("e1000e:
> > > > Refactor PM
> > > > flows"), but it may be a later patch that introduced the non-
> > > > recoverable
> > > > behaviour.
> > > >
> > > > Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > > Cc: Dave Ertman <davidx.m.ertman@intel.com>
> > > > Cc: Bruce Allan <bruce.w.allan@intel.com>
> > > > Cc: intel-wired-lan@lists.osuosl.org
> > > > Cc: netdev@vger.kernel.org
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > [Jani: bikeshed repainted]
> > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > >
> > > Jeff, please make sure this gets submitted to me soon.
> > 
> > Expect it later tonight, just finishing up testing.
> 
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>

Hm, I seem to be blind, but I can't find it anywhere in -rc6. Does someone
have the sha1 from Linus' git for this patch?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

[Dave Airlie]

On 20 June 2017 at 18:49, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Jun 07, 2017 at 01:07:33AM +0000, Brown, Aaron F wrote:
>> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
>> > Of Jeff Kirsher
>> > Sent: Tuesday, June 6, 2017 1:46 PM
>> > To: David Miller <davem@davemloft.net>; Nikula, Jani
>> > <jani.nikula@intel.com>
>> > Cc: Ursulin, Tvrtko <tvrtko.ursulin@intel.com>; daniel.vetter@ffwll.ch; intel-
>> > gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
>> > jani.nikula@linux.intel.com; chris@chris-wilson.co.uk; Ertman, David M
>> > <david.m.ertman@intel.com>; intel-wired-lan@lists.osuosl.org; dri-
>> > devel@lists.freedesktop.org; netdev@vger.kernel.org; airlied@gmail.com
>> > Subject: Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo
>> > e1000e_pm_freeze if __e1000_shutdown fails
>> >
>> > On Fri, 2017-06-02 at 14:14 -0400, David Miller wrote:
>> > > From: Jani Nikula <jani.nikula@intel.com>
>> > > Date: Wed, 31 May 2017 18:50:43 +0300
>> > >
>> > > > From: Chris Wilson <chris@chris-wilson.co.uk>
>> > > >
>> > > > An error during suspend (e100e_pm_suspend),
>> > >
>> > >  ...
>> > > > lead to complete failure:
>> > >
>> > >  ...
>> > > > The unwind failures stems from commit 2800209994f8 ("e1000e:
>> > > > Refactor PM
>> > > > flows"), but it may be a later patch that introduced the non-
>> > > > recoverable
>> > > > behaviour.
>> > > >
>> > > > Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
>> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
>> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> > > > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> > > > Cc: Dave Ertman <davidx.m.ertman@intel.com>
>> > > > Cc: Bruce Allan <bruce.w.allan@intel.com>
>> > > > Cc: intel-wired-lan@lists.osuosl.org
>> > > > Cc: netdev@vger.kernel.org
>> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > > > [Jani: bikeshed repainted]
>> > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> > >
>> > > Jeff, please make sure this gets submitted to me soon.
>> >
>> > Expect it later tonight, just finishing up testing.
>>
>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>
> Hm, I seem to be blind, but I can't find it anywhere in -rc6. Does someone
> have the sha1 from Linus' git for this patch?

Guys this is a pretty serious regression, just left blowing in the
wind, is anyone responsible for e1000e?

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

[Jeff Kirsher]

[-- Attachment #1.1: Type: text/plain, Size: 2907 bytes --]

On Wed, 2017-06-28 at 05:28 +1000, Dave Airlie wrote:
> On 20 June 2017 at 18:49, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Jun 07, 2017 at 01:07:33AM +0000, Brown, Aaron F wrote:
> > > > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org]
> > > > On Behalf
> > > > Of Jeff Kirsher
> > > > Sent: Tuesday, June 6, 2017 1:46 PM
> > > > To: David Miller <davem@davemloft.net>; Nikula, Jani
> > > > <jani.nikula@intel.com>
> > > > Cc: Ursulin, Tvrtko <tvrtko.ursulin@intel.com>; daniel.vetter@ffwll
> > > > .ch; intel-
> > > > gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> > > > jani.nikula@linux.intel.com; chris@chris-wilson.co.uk; Ertman,
> > > > David M
> > > > <david.m.ertman@intel.com>; intel-wired-lan@lists.osuosl.org; dri-
> > > > devel@lists.freedesktop.org; netdev@vger.kernel.org; airlied@gmail.
> > > > com
> > > > Subject: Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo
> > > > e1000e_pm_freeze if __e1000_shutdown fails
> > > > 
> > > > On Fri, 2017-06-02 at 14:14 -0400, David Miller wrote:
> > > > > From: Jani Nikula <jani.nikula@intel.com>
> > > > > Date: Wed, 31 May 2017 18:50:43 +0300
> > > > > 
> > > > > > From: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > 
> > > > > > An error during suspend (e100e_pm_suspend),
> > > > > 
> > > > >  ...
> > > > > > lead to complete failure:
> > > > > 
> > > > >  ...
> > > > > > The unwind failures stems from commit 2800209994f8 ("e1000e:
> > > > > > Refactor PM
> > > > > > flows"), but it may be a later patch that introduced the non-
> > > > > > recoverable
> > > > > > behaviour.
> > > > > > 
> > > > > > Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
> > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
> > > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > > > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > > > > Cc: Dave Ertman <davidx.m.ertman@intel.com>
> > > > > > Cc: Bruce Allan <bruce.w.allan@intel.com>
> > > > > > Cc: intel-wired-lan@lists.osuosl.org
> > > > > > Cc: netdev@vger.kernel.org
> > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > [Jani: bikeshed repainted]
> > > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > > > > 
> > > > > Jeff, please make sure this gets submitted to me soon.
> > > > 
> > > > Expect it later tonight, just finishing up testing.
> > > 
> > > Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> > 
> > Hm, I seem to be blind, but I can't find it anywhere in -rc6. Does
> > someone
> > have the sha1 from Linus' git for this patch?
> 
> Guys this is a pretty serious regression, just left blowing in the
> wind, is anyone responsible for e1000e?

This was submitted and accepted into David Miller's net-next tree.  I can
see if Dave can pull it into his net tree.  DOes stable need to pick this
up as well?

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

[Daniel Vetter]

On Tue, Jun 27, 2017 at 10:51 PM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> This was submitted and accepted into David Miller's net-next tree.  I can
> see if Dave can pull it into his net tree.  DOes stable need to pick this
> up as well?

Nah if it landed somewhere at least I'm happy, we can carry the fixup
for a while longer locally.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

[Neftin, Sasha]

On 5/31/2017 18:50, Jani Nikula wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> An error during suspend (e100e_pm_suspend),
>
> [  429.994338] ACPI : EC: event blocked
> [  429.994633] e1000e: EEE TX LPI TIMER: 00000011
> [  430.955451] pci_pm_suspend(): e1000e_pm_suspend+0x0/0x30 [e1000e] returns -2
> [  430.955454] dpm_run_callback(): pci_pm_suspend+0x0/0x140 returns -2
> [  430.955458] PM: Device 0000:00:19.0 failed to suspend async: error -2
> [  430.955581] PM: Some devices failed to suspend, or early wake event detected
> [  430.957709] ACPI : EC: event unblocked
>
> lead to complete failure:
>
> [  432.585002] ------------[ cut here ]------------
> [  432.585013] WARNING: CPU: 3 PID: 8372 at kernel/irq/manage.c:1478 __free_irq+0x9f/0x280
> [  432.585015] Trying to free already-free IRQ 20
> [  432.585016] Modules linked in: cdc_ncm usbnet x86_pkg_temp_thermal intel_powerclamp coretemp mii crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep lpc_ich snd_hda_core snd_pcm mei_me mei sdhci_pci sdhci i915 mmc_core e1000e ptp pps_core prime_numbers
> [  432.585042] CPU: 3 PID: 8372 Comm: kworker/u16:40 Tainted: G     U          4.10.0-rc8-CI-Patchwork_3870+ #1
> [  432.585044] Hardware name: LENOVO 2356GCG/2356GCG, BIOS G7ET31WW (1.13 ) 07/02/2012
> [  432.585050] Workqueue: events_unbound async_run_entry_fn
> [  432.585051] Call Trace:
> [  432.585058]  dump_stack+0x67/0x92
> [  432.585062]  __warn+0xc6/0xe0
> [  432.585065]  warn_slowpath_fmt+0x4a/0x50
> [  432.585070]  ? _raw_spin_lock_irqsave+0x49/0x60
> [  432.585072]  __free_irq+0x9f/0x280
> [  432.585075]  free_irq+0x34/0x80
> [  432.585089]  e1000_free_irq+0x65/0x70 [e1000e]
> [  432.585098]  e1000e_pm_freeze+0x7a/0xb0 [e1000e]
> [  432.585106]  e1000e_pm_suspend+0x21/0x30 [e1000e]
> [  432.585113]  pci_pm_suspend+0x71/0x140
> [  432.585118]  dpm_run_callback+0x6f/0x330
> [  432.585122]  ? pci_pm_freeze+0xe0/0xe0
> [  432.585125]  __device_suspend+0xea/0x330
> [  432.585128]  async_suspend+0x1a/0x90
> [  432.585132]  async_run_entry_fn+0x34/0x160
> [  432.585137]  process_one_work+0x1f4/0x6d0
> [  432.585140]  ? process_one_work+0x16e/0x6d0
> [  432.585143]  worker_thread+0x49/0x4a0
> [  432.585145]  kthread+0x107/0x140
> [  432.585148]  ? process_one_work+0x6d0/0x6d0
> [  432.585150]  ? kthread_create_on_node+0x40/0x40
> [  432.585154]  ret_from_fork+0x2e/0x40
> [  432.585156] ---[ end trace 6712df7f8c4b9124 ]---
>
> The unwind failures stems from commit 2800209994f8 ("e1000e: Refactor PM
> flows"), but it may be a later patch that introduced the non-recoverable
> behaviour.
>
> Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Dave Ertman <davidx.m.ertman@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> [Jani: bikeshed repainted]
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index b3679728caac..5cad688be609 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -6630,12 +6630,17 @@ static int e1000e_pm_thaw(struct device *dev)
>   static int e1000e_pm_suspend(struct device *dev)
>   {
>   	struct pci_dev *pdev = to_pci_dev(dev);
> +	int rc;
>   
>   	e1000e_flush_lpic(pdev);
>   
>   	e1000e_pm_freeze(dev);
>   
> -	return __e1000_shutdown(pdev, false);
> +	rc = __e1000_shutdown(pdev, false);
> +	if (rc)
> +		e1000e_pm_thaw(dev);
> +
> +	return rc;
>   }
>   
>   static int e1000e_pm_resume(struct device *dev)

Good. Let's pick up this patch.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PULL] topic/e1000e-fix

[Daniel Vetter]

On Wed, May 31, 2017 at 5:08 PM, David Miller <davem@davemloft.net> wrote:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Wed, 31 May 2017 08:10:45 +0200
>
>> On Wed, May 31, 2017 at 7:54 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>> On Wed, May 31, 2017 at 1:06 AM, Dave Airlie <airlied@gmail.com> wrote:
>>>> On 31 May 2017 at 08:10, David Miller <davem@davemloft.net> wrote:
>>>>> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> Date: Tue, 30 May 2017 22:15:42 +0200
>>>>>
>>>>>> If the e1000e maintainer wants to coalesce or not return statements
>>>>>> this simple way, that's imo on him to change the color as needed.
>>>>>
>>>>> That's not how things work.
>>>>>
>>>>> If the maintainer wants you to style things a certain way, either you
>>>>> do it that way or your patch isn't accepted.
>>>
>>> Consider this pull a regression report, pls handle it.
>>
>> And I guess I pile of more cc, to make this regression report
>> complete. I mean you got the backtrace, bisect and a proposed fix, and
>> the almost-whitespace change demanded is something gcc does in its
>> sleep. I'd understand a request to retest if it would be a real
>> functional change, but in this situation I have no idea why this
>> regression just can't be fixed already.
>
> And we can't understand why respinning with the requested change is
> less work than making several postings such as this one.

I guess next time around we should do even less, i.e. report the
regression + bisect and then escalate this until the netdev folks fix
it on their own?

Like Jani said every -rc1 a pile of our CI machines keel over, and
unfortunately this patch here isn't the only one which seems to not
really move anywhere.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel