netdev - Re: [PATCH 3/3] thermal: intel: hfi: Enable interface only when required

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240201162125.GA20261@ranerica-svr.sc.intel.com>
Date: Thu, 1 Feb 2024 08:21:25 -0800
From: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
To: Stanislaw Gruszka <stanislaw.gruszka@...ux.intel.com>
Cc: linux-pm@...r.kernel.org, "Rafael J. Wysocki" <rafael@...nel.org>,
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH 3/3] thermal: intel: hfi: Enable interface only when
 required

On Thu, Feb 01, 2024 at 02:20:04PM +0100, Stanislaw Gruszka wrote:
> On Wed, Jan 31, 2024 at 05:48:08PM -0800, Ricardo Neri wrote:
> > >  drivers/thermal/intel/intel_hfi.c | 82 +++++++++++++++++++++++++++----
> > >  1 file changed, 73 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c
> > > index 3b04c6ec4fca..50601f75f6dc 100644
> > > --- a/drivers/thermal/intel/intel_hfi.c
> > > +++ b/drivers/thermal/intel/intel_hfi.c
> > > @@ -30,6 +30,7 @@
> > >  #include <linux/kernel.h>
> > >  #include <linux/math.h>
> > >  #include <linux/mutex.h>
> > > +#include <linux/netlink.h>
> > >  #include <linux/percpu-defs.h>
> > >  #include <linux/printk.h>
> > >  #include <linux/processor.h>
> > > @@ -480,7 +481,8 @@ void intel_hfi_online(unsigned int cpu)
> > >  	/* Enable this HFI instance if this is its first online CPU. */
> > >  	if (cpumask_weight(hfi_instance->cpus) == 1) {
> > >  		hfi_set_hw_table(hfi_instance);
> > > -		hfi_enable();
> > > +		if (thermal_group_has_listeners(THERMAL_GENL_EVENT_GROUP))
> > > +			hfi_enable();
> > 
> > You could avoid the extra level of indentation if you did:
> > 	if (cpumask_weight() == 1 && has_listeners())
> 
> Ok.
> 
> > You would need to also update the comment.
> 
> I'd rather remove the comment, code looks obvious enough for me.

Do you think that it is clar that needs to be done only once per
package? I guess it is clear but only after reading the more code.