From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754619AbcKPA7u (ORCPT ); Tue, 15 Nov 2016 19:59:50 -0500 Received: from mout.gmx.net ([212.227.17.22]:57540 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751985AbcKPA7s (ORCPT ); Tue, 15 Nov 2016 19:59:48 -0500 Date: Wed, 16 Nov 2016 01:59:30 +0100 From: Kast Bernd To: David Miller Cc: nic_swsd@realtek.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] r8169: Module parameter for opt-in of ASPM Message-ID: <20161116005930.GA32205@ASUS-Bernd.fritz.box> References: <20161103233306.GA9368@ASUS-Bernd.fritz.box> <20161112.210224.1889131711533608896.davem@davemloft.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161112.210224.1889131711533608896.davem@davemloft.net> User-Agent: Mutt/1.7.1 (2016-10-04) X-Provags-ID: V03:K0:Ph1CdwTA6ia/eGiXWD6OfCRUfsWKFJrjurpiDH7Lqr6zfBRG0+m wAf/wn0GmGTi5L1GsI8ut5tPvMUybufKOfGOc4u2EoOly3OPIBfz6YcNRMz9XqeE0cTg468 EhlEtWRa/JJ3iSL/xQG+FjjDyAO0foyJqJNkjzWrK1OK9j5mKZ9Lg/X85U2rna7KfxISlY/ OPyqgIURLVPWx8et6nicw== X-UI-Out-Filterresults: notjunk:1;V01:K0:5lyBsLAQgCs=:Acl2caVcfu6jSSRRpW0C5E IihWV4yXH9jXBHsxQ5depQ0pusJ0XewIVbxIS/m2mjp+03UEKN42ctMQg8oblz49aRBb7Qa1A 2GGGFMgeyxg0rFboyMb0PWBpSu5QvlK+EMf7oZZyJUrNgt8Oxj+mKYHT+579DUulZLpDQVuTi iiaPWTdsqAGhVsTAlYbHChhpiw0dtdmE6CNE/7co7AJbax+2zccrfCgfVIB6/CJf1aLbTYWgX 4YnwbytW3zjWh5v9wf5LOXhw+E5cAcrHTHBHsTuQxkl7zVZYEIO3AuRGQeptHDTPKxs+d3Q4/ x83+p3T46WJnxVKET00GzwyAs7iedl/n58SXbj3OM0Q4Bh4oYRYfg81LBZAllMprzsBrSR1iE EwBL4Ju7l8UXIBjYZe52FNDSoENhpmpLBVc1aM2Rce4LDid5qtKJDV+OPKlChMCxmljxbSg58 wSiTjucqyLbOc9kcbOpzUbpCfqXrtIKd9YUeck79MUVhsv5AyBwIXnpcwrQt5mUefzFDMdGpm CtfU71BXlhqUG3c9NIxbip7AMfA7xxWifvXvjp2gruJOgW8amPid1rPHhRWV1aGsNy/aABcbR h2F9MtTIjHkSYP/p7Z7AMNANq2NuCcbA7MlmudbgwOHHNlC0MVR66R39tfKGPtycg7fceVl05 jpu2DInN+n2Eoq13jXhzLwKcrbAQy8c8WcDkyuNMRQqUEO3fCq+NMNchhpGBGDvaZ0lCmu/4v 5zOOnjq3QwV9hm98TlWOCl2tHTMqiXTE7B1zbHFvpvZ5kvrjbeKhhsKh6sQ= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Nov 12, 2016 at 09:02:24PM -0500, David Miller wrote: > From: Kast Bernd > Date: Fri, 4 Nov 2016 00:33:06 +0100 > > > This patch adds a module parameter in order to activate ASPM. By that > > the CPU can enter deep sleep modes (PC6) and power consumption can be > > reduced (for example from 13W to 8W on my notebook with a Haswell CPU). > > Basically, it reapplies d64ec841517a25f6d468bde9f67e5b4cffdc67c7, which > > was reverted due to delayed link status detection and increased boot > > times on some systems. These bugs are avoided by two actions: > > 1) ASPM is turned off by default to avoid any problems with the > > default configuration. > > 2) Flags for ASPM and clock request are set after ephy_init, > > which wasn't respected on the previous patch. Thus ASPM with > > this patch could work even with previously failing systems. > > This feels like grasping at straws. > > If you feel you've corrected a flaw in the previous ASPM > support, then let's reinstate it unconditionally without > some module parameter. > > We'll never find out if you actually did fix ASPM support > sufficiently if it's off by default. > > Only experts like you will ever enable the option. It's > usage will be minimal, and therefore the benefits will not > be sufficient to justify these changes in the first place. The correction of that flaw is rather a side effect of reapplying the old patch. Perhaps it will allow enabling ASPM on some additional systems, but for sure it won't solve all ASPM related problems. I know that the module parameters are frowned upon, as Francois Romieu stated. Nevertheless, nobody commented on his suggestion to overcome the problem of disabled ASPM (1). I would really love to see ASPM enabled by default. However, it seems to be quite unlikely, as it has the potential to trigger bugs and freezes on some systems. I can understand, that the main focus of the kernel is stability. Nonetheless, I also mind power consumption. That's why I used this kernel parameter to provide a simple solution, which can significantly reduce power consumption and heat production without affecting stability. If we can achieve that without a kernel parameter it's perfectly fine for me. From my point of view, there are only two other solutions: -a white list, like proposed by Francois Romieu, that enables ASPM on systems, that are known to work stable -a black list, that disables ASPM on buggy systems While the first solution results in lots of work to keep that list up to date, the second one will lead to unstable systems. I don't know how many systems are out there, that suffer from problems that are really caused by ASPM and not by some other bug,as even the commit (2), that disabled ASPM in general for r8169, cites a bug report, that doensn't seem to be related to ASPM at all (3). However, others state, that disabling ASPM solved their problems (4). Finding the flaws that caused these bugs or listing these systems is beyond my possibilities. Thus the kernel paramter, although it is not perfect, seemed to be the best compromise to me. (1) http://lkml.iu.edu/hypermail/linux/kernel/1605.1/04992.html (2) 4521e1a94279ce610d3f9b7945c17d581f804242 (3) https://bugzilla.redhat.com/show_bug.cgi?id=642861#c9 (4) https://bugzilla.redhat.com/show_bug.cgi?id=538920