diff mbox series

[v3,1/8] opal: Update opal_del_host_sync_notifier() to accept 'void *data'

Message ID 20190108095902.24718-2-vaibhav@linux.ibm.com
State Superseded
Headers show
Series Enable fast-reboot support for CAPI-2 | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master

Commit Message

Vaibhav Jain Jan. 8, 2019, 9:58 a.m. UTC 
Current implementation of opal_del_host_sync_notifier() will only
delete the first entry of the 'notify' callback found from opal_syncers
list irrespective of the 'data' of list-node. This is problematic when
multiple notifiers with same callback function but different 'data'
are registered. In this case when the cleanup code will call
opal_del_host_sync_notifier() it cannot be sure if correct opal_syncer
is removed.

Hence this patch updates the function to accept a new argument named
'void *data' which is then used to iterates over the opal_syncers list
and only remove the first node node having the matching value for
'notify' callback as 'data'.

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Reviewed-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Change-log

v3:	None/Respining the patchset

v2: 	Instead of introducing a new function, update the existing
	function opal_del_host_sync_notifier() to accept 'void *data' [Vasant]
---
 core/opal.c             | 7 +++++--
 include/opal-internal.h | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

Frederic Barrat Jan. 10, 2019, 1:37 p.m. UTC | #1 
Le 08/01/2019 à 10:58, Vaibhav Jain a écrit :
> Current implementation of opal_del_host_sync_notifier() will only
> delete the first entry of the 'notify' callback found from opal_syncers
> list irrespective of the 'data' of list-node. This is problematic when
> multiple notifiers with same callback function but different 'data'
> are registered. In this case when the cleanup code will call
> opal_del_host_sync_notifier() it cannot be sure if correct opal_syncer
> is removed.
> 
> Hence this patch updates the function to accept a new argument named
> 'void *data' which is then used to iterates over the opal_syncers list
> and only remove the first node node having the matching value for
> 'notify' callback as 'data'.
> 
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Reviewed-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Change-log
> 
> v3:	None/Respining the patchset
> 
> v2: 	Instead of introducing a new function, update the existing
> 	function opal_del_host_sync_notifier() to accept 'void *data' [Vasant]
> ---

Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

>   core/opal.c             | 7 +++++--
>   include/opal-internal.h | 2 +-
>   2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/core/opal.c b/core/opal.c
> index 3a8ea30b..87aca61c 100644
> --- a/core/opal.c
> +++ b/core/opal.c
> @@ -683,12 +683,15 @@ void opal_add_host_sync_notifier(bool (*notify)(void *data), void *data)
>   	list_add_tail(&opal_syncers, &ent->link);
>   }
>   
> -void opal_del_host_sync_notifier(bool (*notify)(void *data))
> +/*
> + * Remove a host sync notifier for given callback and data
> + */
> +void opal_del_host_sync_notifier(bool (*notify)(void *data), void *data)
>   {
>   	struct opal_sync_entry *ent;
>   
>   	list_for_each(&opal_syncers, ent, link) {
> -		if (ent->notify == notify) {
> +		if (ent->notify == notify && ent->data == data) {
>   			list_del(&ent->link);
>   			free(ent);
>   			return;
> diff --git a/include/opal-internal.h b/include/opal-internal.h
> index 40bad457..2ce25adb 100644
> --- a/include/opal-internal.h
> +++ b/include/opal-internal.h
> @@ -76,7 +76,7 @@ extern void opal_run_pollers(void);
>    * Warning: no locking, only call that from the init processor
>    */
>   extern void opal_add_host_sync_notifier(bool (*notify)(void *data), void *data);
> -extern void opal_del_host_sync_notifier(bool (*notify)(void *data));
> +extern void opal_del_host_sync_notifier(bool (*notify)(void *data), void *data);
>   
>   /*
>    * Opal internal function prototype
>
diff mbox series

Patch

diff --git a/core/opal.c b/core/opal.c
index 3a8ea30b..87aca61c 100644
--- a/core/opal.c
+++ b/core/opal.c
@@ -683,12 +683,15 @@  void opal_add_host_sync_notifier(bool (*notify)(void *data), void *data)
 	list_add_tail(&opal_syncers, &ent->link);
 }
 
-void opal_del_host_sync_notifier(bool (*notify)(void *data))
+/*
+ * Remove a host sync notifier for given callback and data
+ */
+void opal_del_host_sync_notifier(bool (*notify)(void *data), void *data)
 {
 	struct opal_sync_entry *ent;
 
 	list_for_each(&opal_syncers, ent, link) {
-		if (ent->notify == notify) {
+		if (ent->notify == notify && ent->data == data) {
 			list_del(&ent->link);
 			free(ent);
 			return;
diff --git a/include/opal-internal.h b/include/opal-internal.h
index 40bad457..2ce25adb 100644
--- a/include/opal-internal.h
+++ b/include/opal-internal.h
@@ -76,7 +76,7 @@  extern void opal_run_pollers(void);
  * Warning: no locking, only call that from the init processor
  */
 extern void opal_add_host_sync_notifier(bool (*notify)(void *data), void *data);
-extern void opal_del_host_sync_notifier(bool (*notify)(void *data));
+extern void opal_del_host_sync_notifier(bool (*notify)(void *data), void *data);
 
 /*
  * Opal internal function prototype