Re: [PATCH 3/3] x86/mce/therm_throt: allow disabling the thermal vector altogether

From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Borislav Petkov <bp@suse.de>, LKML <linux-kernel@vger.kernel.org>,
	linux-edac@vger.kernel.org, X86 ML <x86@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	bberg@redhat.com, Tony Luck <tony.luck@intel.com>
Subject: Re: [PATCH 3/3] x86/mce/therm_throt: allow disabling the thermal vector altogether
Date: Tue, 14 Apr 2020 14:51:22 -0700	[thread overview]
Message-ID: <b64aa350ba40c53e799690141006e0f1885d28a2.camel@linux.intel.com> (raw)
In-Reply-To: <CAHmME9rx6w=p6+AFP=0Y5tPEq1CxUTGe4h8SXz-qRNuCvU0hyw@mail.gmail.com>

On Tue, 2020-04-14 at 15:07 -0600, Jason A. Donenfeld wrote:
> On Tue, Apr 14, 2020 at 2:49 PM Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > On Tue, 2020-04-14 at 22:23 +0200, Borislav Petkov wrote:
> > > + Tony.
> > > 
> > > On Tue, Apr 14, 2020 at 01:41:08PM -0600, Jason A. Donenfeld
> > > wrote:
> > > > Generally it is desirable, and extremely common too. This
> > > > thermal
> > > > code
> > > > -- which mostly functions to print some messages into kmsg --
> > > > is
> > > > very
> > > > verbose. This is not something I want to compile into smaller
> > > > systems.
> > > > This is the reason why kconfig has options in the first place.
> > > > I'm
> > > > not
> > > > sure yet-another boottime flag makes sense for this.
> > > 
> > > I don't mind making the already existing option selectable and
> > > leaving
> > > it default y, i.e., keeping the current situation by default. And
> > > people
> > > who want to disable it, can then do so.
> > > 
> > > I do mind to having yet another config option though. No thanks -
> > > they're already too many.
> > > 
> > > So it should be an all or nothing thing.
> > > 
> > > Poking quickly at that and
> > > drivers/thermal/intel/x86_pkg_temp_thermal.c,
> > > all those things do is report trip points. therm_throt reports
> > > how
> > > long
> > > the hw throttled due to hitting a trip point, etc.
> > > 
> > > IINM, of course so please correct me if I'm missing anything.
> > > 
> > > But if not and this all is only for reporting and doesn't have
> > > any
> > > detrimental effects on the hardware when missing from the system,
> > > then I
> > > guess we could make CONFIG_X86_THERMAL_VECTOR user-selectable.
> > 
> > We can make user selectable
> > 
> > These drivers are used for reporting only.
> > User space can select a trip temperature via x86_pkg_temp and get
> > notification via uevent to start additional cooling system
> > (additional
> 
> I didn't see any uevent stuff. Is this part of out-of-tree modules or
> proprietary code that's hooking into those EXPORT_SYMBOL (non-GPL)
> exports?
This is not out of tree. This is x86_pkg_temp driver as part of thermal
subsystem, and thermal_zone_device_update()  user space governor issues uevent. But those are different modifiable thresholds not the high/low temperature thresholds.


> 
> > fans, liquid coooling etc), so that processor don't have to go self
> > throttling mode. Self throttling depending on processor series and
> > firmware can be very aggressive.
> > In client systems thermald will set a temperature and starts power
> > control once it reaches passive temperature limit. But it can
> > function
> > without x86_pkg_temp also, so even if user disables thermal
> > reporting
> > it can still function.
> 
> The 2/3 patch may be interesting as well to you. This removes the
> expensive work queue management stuff if the option is deselected,
> since all those workqueues do is print messages to kmsg, while
> retaining the rest of the infra.