Re: [PATCH v2 04/19] libmpathutil: add implementation of generic shared pointer

On 5/22/26 18:44, Martin Wilck wrote:
> Add a set of simple functions to handle refcounted pointers.
> 
> Signed-off-by: Martin Wilck <[email protected]>
> Reviewed-by: Benjamin Marzinski <[email protected]>
> ---
>   libmpathutil/libmpathutil.version |  7 +++++
>   libmpathutil/util.c               | 45 ++++++++++++++++++++++++++++---
>   libmpathutil/util.h               |  5 ++++
>   3 files changed, 53 insertions(+), 4 deletions(-)
> 
That really looks like the Hazard pointers from Paul McK and Mathieu 
Desnoyers. Maybe have a look and see if you cannot re-use the
approach here.

> diff --git a/libmpathutil/libmpathutil.version b/libmpathutil/libmpathutil.version
> index c69120d..51594ad 100644
> --- a/libmpathutil/libmpathutil.version
> +++ b/libmpathutil/libmpathutil.version
> @@ -194,3 +194,10 @@ global:
>   local:
>   	*;
>   };
> +
> +LIBMPATHUTIL_6.1 {
> +global:
> +	alloc_shared_ptr;
> +	get_shared_ptr;
> +	put_shared_ptr;
> +} LIBMPATHUTIL_6.0;
> diff --git a/libmpathutil/util.c b/libmpathutil/util.c
> index 23a9797..3c623ec 100644
> --- a/libmpathutil/util.c
> +++ b/libmpathutil/util.c
> @@ -12,15 +12,13 @@
>   #include <dirent.h>
>   #include <unistd.h>
>   #include <errno.h>
> +#include <urcu/uatomic.h>
>   #include "mt-udev-wrap.h"
>   
>   #include "util.h"
>   #include "debug.h"
> -#include "checkers.h"
>   #include "vector.h"
> -#include "structs.h"
> -#include "config.h"
> -#include "log.h"
> +#include "list.h"
>   
>   size_t
>   strchop(char *str)
> @@ -384,3 +382,42 @@ void cleanup_udev_device(struct udev_device **udd)
>   	if (*udd)
>   		udev_device_unref(*udd);
>   }
> +
> +struct shared_ptr {
> +	long refcnt;
> +	void (*destructor)(void *);
> +	char __attribute__((aligned(sizeof(void *)))) ptr[];
> +};
> +
> +void *alloc_shared_ptr(size_t size, void (*destructor)(void *))
> +{
> +	struct shared_ptr *sp = malloc(sizeof(*sp) + size);
> +
> +	if (!sp)
> +		return NULL;
> +	uatomic_set(&sp->refcnt, 1);
> +	sp->destructor = destructor;
> +	return sp->ptr;
> +}
> +
> +void get_shared_ptr(void *ptr)
> +{
> +	struct shared_ptr *sp = container_of(ptr, struct shared_ptr, ptr);
> +
> +	if (uatomic_add_return(&sp->refcnt, 1) < 0)
> +		condlog(0, "%s: refcount overflow", __func__);
> +}
> +
> +void put_shared_ptr(void *ptr)
> +{
> +	struct shared_ptr *sp;
> +
> +	if (!ptr)
> +		return;
> +	sp = container_of(ptr, struct shared_ptr, ptr);
> +	if (uatomic_sub_return(&sp->refcnt, 1) == 0) {
> +		if (sp->destructor)
> +			sp->destructor(ptr);
> +		free(sp);
> +	}
> +}

I _think_ the could be implemented better using RCU primitives;
in the end, what you really care about is whether the pointer is
valid (9which is what RCU provides).
Or indeed use the hazard pointer approach, and count the number
of RCU references to that object ...

> diff --git a/libmpathutil/util.h b/libmpathutil/util.h
> index aed1bc1..ffbb182 100644
> --- a/libmpathutil/util.h
> +++ b/libmpathutil/util.h
> @@ -160,4 +160,9 @@ void cleanup_charp(char **p);
>   void cleanup_ucharp(unsigned char **p);
>   void cleanup_udev_device(struct udev_device **udd);
>   void cleanup_bitfield(union bitfield **p);
> +
> +void *alloc_shared_ptr(size_t size, void (*destructor)(void *));
> +void get_shared_ptr(void *ptr);
> +void put_shared_ptr(void *ptr);
> +
>   #endif /* UTIL_H_INCLUDED */

... or just leave it as it is if the approach is to complex.
I doubt it's time-critical.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
[email protected]                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich