2024-03-27 21:28:58

by Johannes Berg

Subject: Re: [PATCH 0/3] using guard/__free in networking

On Wed, 2024-03-27 at 21:25 +0100, Jakub Sitnicki wrote:
> On Tue, Mar 26, 2024 at 04:33 PM +01, Johannes Berg wrote:
> > > Is it also present in Rust or some such?
> >
> > I have no idea. I _think_ Rust actually ties the data and the locks
> > together more?
>
> That is right. Nicely explained here:
>
> https://marabos.nl/atomics/basics.html#rusts-mutex

Right.

Thinking about that, we _could_ even add support for drop_guard()?

With the below patch to cleanup.h, you can write

void my_something(my_t *my)
{
..
named_guard(lock, mutex)(&my->mutex);
..
if (foo)
return -EINVAL; // automatically unlocks
..
// no need for lock any more
drop_guard(lock);
..
// do other things now unlocked
}


Is that too ugly? Maybe it's less Java-like and more Rust-like and
better for Jakub ;-)

In some sense that's nicer than scoped_guard() since it doesn't require
a new scope / indentation, but maybe Peter already thought about it and
rejected it :-)


Patch follows, though maybe that should be rolled into the 'base' CLASS
definition instead of defining another "_drop" one for named_guard()?

diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..31298106c28b 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -106,7 +106,27 @@ typedef _type class_##_name##_t; \
static inline void class_##_name##_destructor(_type *p) \
{ _type _T = *p; _exit; } \
static inline _type class_##_name##_constructor(_init_args) \
-{ _type t = _init; return t; }
+{ _type t = _init; return t; } \
+typedef struct class_##_name##_drop##_t { \
+ class_##_name##_t obj; \
+ void (*destructor)(struct class_##_name##_drop##_t *); \
+} class_##_name##_drop##_t; \
+static inline void \
+class_##_name##_drop##_destructor(class_##_name##_drop##_t *p) \
+{ \
+ if (p->obj) \
+ class_##_name##_destructor(&p->obj); \
+ p->obj = NULL; \
+} \
+static inline class_##_name##_drop##_t \
+class_##_name##_drop##_constructor(_init_args) \
+{ \
+ class_##_name##_drop##_t t = { \
+ .obj = _init, \
+ .destructor = class_##_name##_drop##_destructor, \
+ }; \
+ return t; \
+}

#define EXTEND_CLASS(_name, ext, _init, _init_args...) \
typedef class_##_name##_t class_##_name##ext##_t; \
@@ -163,6 +183,11 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
#define guard(_name) \
CLASS(_name, __UNIQUE_ID(guard))

+#define named_guard(_name, _class) \
+ CLASS(_class##_drop, _name)
+
+#define drop_guard(_name) do { _name.destructor(&_name); } while (0)
+
#define __guard_ptr(_name) class_##_name##_lock_ptr

#define scoped_guard(_name, args...) \

2024-03-27 21:43:48

by Johannes Berg

Subject: Re: [PATCH 0/3] using guard/__free in networking

On Wed, 2024-03-27 at 22:28 +0100, Johannes Berg wrote:
>
> +typedef struct class_##_name##_drop##_t { \
> + class_##_name##_t obj; \
> + void (*destructor)(struct class_##_name##_drop##_t *); \
> +} class_##_name##_drop##_t; \

No, I misread the compiler output, it does output a real destructor
function to push it into a stack variable with this...

So I guess it'd have to be

void my_something(my_t *my)
{
..
named_guard(lock, mutex)(&my->mutex);
..
if (foo)
return -EINVAL; // automatically unlocks
..
// no need for lock any more
drop_guard(lock, mutex);
..
// do other things now unlocked
}


instead, syntax-wise.


Which obviously simplifies the changes:


diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..cf39a4a3f56f 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -163,6 +163,12 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
#define guard(_name) \
CLASS(_name, __UNIQUE_ID(guard))

+#define named_guard(_name, _class) \
+ CLASS(_class, _name)
+
+#define drop_guard(_name, _class) \
+ do { class_##_class##_destructor(&_name); _name = NULL; } while (0)
+
#define __guard_ptr(_name) class_##_name##_lock_ptr

#define scoped_guard(_name, args...) \