public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] thermal: netlink: Add enum for mutlicast groups indexes
@ 2023-12-27 14:00 Stanislaw Gruszka
  2023-12-27 14:00 ` [PATCH 2/2] thermal: netlink: Add thermal_group_has_listeners() helper Stanislaw Gruszka
  0 siblings, 1 reply; 6+ messages in thread
From: Stanislaw Gruszka @ 2023-12-27 14:00 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Srinivas Pandruvada, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	linux-pm

Use enum instead of hard-coded numbers for indexing multicast groups.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/thermal/thermal_netlink.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
index 21f00d73acb7..aca36c4ddbf3 100644
--- a/drivers/thermal/thermal_netlink.c
+++ b/drivers/thermal/thermal_netlink.c
@@ -13,9 +13,14 @@
 
 #include "thermal_core.h"
 
+enum thermal_genl_multicast_groups {
+	THERMAL_GENL_SAMPLING_GROUP = 0,
+	THERMAL_GENL_EVENT_GROUP = 1,
+};
+
 static const struct genl_multicast_group thermal_genl_mcgrps[] = {
-	{ .name = THERMAL_GENL_SAMPLING_GROUP_NAME, },
-	{ .name = THERMAL_GENL_EVENT_GROUP_NAME,  },
+	[THERMAL_GENL_SAMPLING_GROUP] = { .name = THERMAL_GENL_SAMPLING_GROUP_NAME, },
+	[THERMAL_GENL_EVENT_GROUP]  = { .name = THERMAL_GENL_EVENT_GROUP_NAME,  },
 };
 
 static const struct nla_policy thermal_genl_policy[THERMAL_GENL_ATTR_MAX + 1] = {
@@ -95,7 +100,7 @@ int thermal_genl_sampling_temp(int id, int temp)
 
 	genlmsg_end(skb, hdr);
 
-	genlmsg_multicast(&thermal_gnl_family, skb, 0, 0, GFP_KERNEL);
+	genlmsg_multicast(&thermal_gnl_family, skb, 0, THERMAL_GENL_SAMPLING_GROUP, GFP_KERNEL);
 
 	return 0;
 out_cancel:
@@ -290,7 +295,7 @@ static int thermal_genl_send_event(enum thermal_genl_event event,
 
 	genlmsg_end(msg, hdr);
 
-	genlmsg_multicast(&thermal_gnl_family, msg, 0, 1, GFP_KERNEL);
+	genlmsg_multicast(&thermal_gnl_family, msg, 0, THERMAL_GENL_EVENT_GROUP, GFP_KERNEL);
 
 	return 0;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] thermal: netlink: Add thermal_group_has_listeners() helper
  2023-12-27 14:00 [PATCH 1/2] thermal: netlink: Add enum for mutlicast groups indexes Stanislaw Gruszka
@ 2023-12-27 14:00 ` Stanislaw Gruszka
  2023-12-27 14:11   ` Daniel Lezcano
  0 siblings, 1 reply; 6+ messages in thread
From: Stanislaw Gruszka @ 2023-12-27 14:00 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Srinivas Pandruvada, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	linux-pm

Add a helper function to check if there are listeners for
thermal_gnl_family multicast groups.

For now use it to avoid unnecessary allocations and sending
thermal genl messages when there are no recipients.

In the future, in conjunction with (not yet implemented) notification
of change in the netlink socket group membership, this helper can be
used to open/close hardware interfaces based on the presence of
user space subscribers.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/thermal/thermal_netlink.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
index aca36c4ddbf3..b4e758d22077 100644
--- a/drivers/thermal/thermal_netlink.c
+++ b/drivers/thermal/thermal_netlink.c
@@ -76,6 +76,11 @@ typedef int (*cb_t)(struct param *);
 
 static struct genl_family thermal_gnl_family;
 
+static int thermal_group_has_listeners(enum thermal_genl_multicast_groups group)
+{
+	return genl_has_listeners(&thermal_gnl_family, &init_net, group);
+}
+
 /************************** Sampling encoding *******************************/
 
 int thermal_genl_sampling_temp(int id, int temp)
@@ -83,6 +88,9 @@ int thermal_genl_sampling_temp(int id, int temp)
 	struct sk_buff *skb;
 	void *hdr;
 
+	if (!thermal_group_has_listeners(THERMAL_GENL_SAMPLING_GROUP))
+		return -ESRCH;
+
 	skb = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!skb)
 		return -ENOMEM;
@@ -280,6 +288,9 @@ static int thermal_genl_send_event(enum thermal_genl_event event,
 	int ret = -EMSGSIZE;
 	void *hdr;
 
+	if (!thermal_group_has_listeners(THERMAL_GENL_EVENT_GROUP))
+		return -ESRCH;
+
 	msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] thermal: netlink: Add thermal_group_has_listeners() helper
  2023-12-27 14:00 ` [PATCH 2/2] thermal: netlink: Add thermal_group_has_listeners() helper Stanislaw Gruszka
@ 2023-12-27 14:11   ` Daniel Lezcano
  2023-12-27 15:31     ` Stanislaw Gruszka
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2023-12-27 14:11 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Rafael J. Wysocki, Srinivas Pandruvada, Zhang Rui, Lukasz Luba,
	linux-pm

Hi Stanislaw,

thanks for this optimization. One question below.

On Wed, Dec 27, 2023 at 03:00:57PM +0100, Stanislaw Gruszka wrote:
> Add a helper function to check if there are listeners for
> thermal_gnl_family multicast groups.
> 
> For now use it to avoid unnecessary allocations and sending
> thermal genl messages when there are no recipients.
> 
> In the future, in conjunction with (not yet implemented) notification
> of change in the netlink socket group membership, this helper can be
> used to open/close hardware interfaces based on the presence of
> user space subscribers.
> 
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
>  drivers/thermal/thermal_netlink.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
> index aca36c4ddbf3..b4e758d22077 100644
> --- a/drivers/thermal/thermal_netlink.c
> +++ b/drivers/thermal/thermal_netlink.c
> @@ -76,6 +76,11 @@ typedef int (*cb_t)(struct param *);
>  
>  static struct genl_family thermal_gnl_family;
>  
> +static int thermal_group_has_listeners(enum thermal_genl_multicast_groups group)
> +{
> +	return genl_has_listeners(&thermal_gnl_family, &init_net, group);
> +}
> +
>  /************************** Sampling encoding *******************************/
>  
>  int thermal_genl_sampling_temp(int id, int temp)
> @@ -83,6 +88,9 @@ int thermal_genl_sampling_temp(int id, int temp)
>  	struct sk_buff *skb;
>  	void *hdr;
>  
> +	if (!thermal_group_has_listeners(THERMAL_GENL_SAMPLING_GROUP))
> +		return -ESRCH;
> +

Do really want to return an error ? Shall we just bail out instead ?

[ ... ]

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] thermal: netlink: Add thermal_group_has_listeners() helper
  2023-12-27 14:11   ` Daniel Lezcano
@ 2023-12-27 15:31     ` Stanislaw Gruszka
  2023-12-27 15:40       ` Daniel Lezcano
  0 siblings, 1 reply; 6+ messages in thread
From: Stanislaw Gruszka @ 2023-12-27 15:31 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Rafael J. Wysocki, Srinivas Pandruvada, Zhang Rui, Lukasz Luba,
	linux-pm

Hi Daniel,

On Wed, Dec 27, 2023 at 03:11:03PM +0100, Daniel Lezcano wrote:
> > +static int thermal_group_has_listeners(enum thermal_genl_multicast_groups group)
> > +{
> > +	return genl_has_listeners(&thermal_gnl_family, &init_net, group);
> > +}
> > +
> >  /************************** Sampling encoding *******************************/
> >  
> >  int thermal_genl_sampling_temp(int id, int temp)
> > @@ -83,6 +88,9 @@ int thermal_genl_sampling_temp(int id, int temp)
> >  	struct sk_buff *skb;
> >  	void *hdr;
> >  
> > +	if (!thermal_group_has_listeners(THERMAL_GENL_SAMPLING_GROUP))
> > +		return -ESRCH;
> > +
> 
> Do really want to return an error ? Shall we just bail out instead ?

I decided for error because thermal_notify_* are int functions and we
return error code for some other cases when the messages can not be
sent (i.e. alloc error).
Event if currently return value is ignored by all callers (FWICT),
error information could be used theoretically. 

If returning 0 is preferable/better, I can change that.

Regards
Stanislaw

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] thermal: netlink: Add thermal_group_has_listeners() helper
  2023-12-27 15:31     ` Stanislaw Gruszka
@ 2023-12-27 15:40       ` Daniel Lezcano
  2023-12-27 16:01         ` Stanislaw Gruszka
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2023-12-27 15:40 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Rafael J. Wysocki, Srinivas Pandruvada, Zhang Rui, Lukasz Luba,
	linux-pm

On 27/12/2023 16:31, Stanislaw Gruszka wrote:
> Hi Daniel,
> 
> On Wed, Dec 27, 2023 at 03:11:03PM +0100, Daniel Lezcano wrote:
>>> +static int thermal_group_has_listeners(enum thermal_genl_multicast_groups group)
>>> +{
>>> +	return genl_has_listeners(&thermal_gnl_family, &init_net, group);
>>> +}
>>> +
>>>   /************************** Sampling encoding *******************************/
>>>   
>>>   int thermal_genl_sampling_temp(int id, int temp)
>>> @@ -83,6 +88,9 @@ int thermal_genl_sampling_temp(int id, int temp)
>>>   	struct sk_buff *skb;
>>>   	void *hdr;
>>>   
>>> +	if (!thermal_group_has_listeners(THERMAL_GENL_SAMPLING_GROUP))
>>> +		return -ESRCH;
>>> +
>>
>> Do really want to return an error ? Shall we just bail out instead ?
> 
> I decided for error because thermal_notify_* are int functions and we
> return error code for some other cases when the messages can not be
> sent (i.e. alloc error).
> Event if currently return value is ignored by all callers (FWICT),
> error information could be used theoretically.
> 
> If returning 0 is preferable/better, I can change that.

The caller will have to handle the specific case if there is no 
listeners. There is an error if the message can not be sent but here 
there is no error related to that, just we don't send it. So having an 
error when there is nothing to do is not really an error.




-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] thermal: netlink: Add thermal_group_has_listeners() helper
  2023-12-27 15:40       ` Daniel Lezcano
@ 2023-12-27 16:01         ` Stanislaw Gruszka
  0 siblings, 0 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2023-12-27 16:01 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Rafael J. Wysocki, Srinivas Pandruvada, Zhang Rui, Lukasz Luba,
	linux-pm

On Wed, Dec 27, 2023 at 04:40:31PM +0100, Daniel Lezcano wrote:
> On 27/12/2023 16:31, Stanislaw Gruszka wrote:
> > Hi Daniel,
> > 
> > On Wed, Dec 27, 2023 at 03:11:03PM +0100, Daniel Lezcano wrote:
> > > > +static int thermal_group_has_listeners(enum thermal_genl_multicast_groups group)
> > > > +{
> > > > +	return genl_has_listeners(&thermal_gnl_family, &init_net, group);
> > > > +}
> > > > +
> > > >   /************************** Sampling encoding *******************************/
> > > >   int thermal_genl_sampling_temp(int id, int temp)
> > > > @@ -83,6 +88,9 @@ int thermal_genl_sampling_temp(int id, int temp)
> > > >   	struct sk_buff *skb;
> > > >   	void *hdr;
> > > > +	if (!thermal_group_has_listeners(THERMAL_GENL_SAMPLING_GROUP))
> > > > +		return -ESRCH;
> > > > +
> > > 
> > > Do really want to return an error ? Shall we just bail out instead ?
> > 
> > I decided for error because thermal_notify_* are int functions and we
> > return error code for some other cases when the messages can not be
> > sent (i.e. alloc error).
> > Event if currently return value is ignored by all callers (FWICT),
> > error information could be used theoretically.
> > 
> > If returning 0 is preferable/better, I can change that.
> 
> The caller will have to handle the specific case if there is no listeners.
> There is an error if the message can not be sent but here there is no error
> related to that, just we don't send it. So having an error when there is
> nothing to do is not really an error.

Ok, will change to 0 in v2.

Regards
Stanislaw

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-12-27 16:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-27 14:00 [PATCH 1/2] thermal: netlink: Add enum for mutlicast groups indexes Stanislaw Gruszka
2023-12-27 14:00 ` [PATCH 2/2] thermal: netlink: Add thermal_group_has_listeners() helper Stanislaw Gruszka
2023-12-27 14:11   ` Daniel Lezcano
2023-12-27 15:31     ` Stanislaw Gruszka
2023-12-27 15:40       ` Daniel Lezcano
2023-12-27 16:01         ` Stanislaw Gruszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox