Re: [PATCH 1/2] cleanup: Add conditional guard support

[Peter Zijlstra]

On Thu, Nov 02, 2023 at 03:40:11PM +0100, Oleg Nesterov wrote:
> On 11/02, Peter Zijlstra wrote:
> >
> >  include/linux/cleanup.h  |   52 ++++++++++++++++++++++++++++++++++++++++++++---
> 
> interesting... I don't know anything about cleanup.h, will
> read this code and the patch later, but I guess I understand
> the idea.
> 
> Stupid/offtopic question... Can't we change guard()
> 
> 	-#define guard(_name) \
> 	-	CLASS(_name, __UNIQUE_ID(guard))
> 	+#define guard(_name, args...) \
> 	+	CLASS(_name, __UNIQUE_ID(guard))(args)
> 
> and update the current users?
> 
> To me
> 
> 	guard(rcu);
> 	guard(spinlock, &lock);
> 
> looks better than
> 
> 	guard(rcu)();
> 	// doesn't match scoped_guard(spinlock, &lock)
> 	guard(spinlock)(&lock);
> 
> And this will make guard() consistent with scoped_guard().
> 
> No?

Yes (and you're not the only one to have noticed), I think an earlier
version actually had that. The current form came about in a fairly long
thread with Linus. Most notably here:

  https://lkml.kernel.org/r/CAHk-%3DwgXN1YxGMUFeuC135aeUvqduF8zJJiZZingzS1Pao5h0A%40mail.gmail.com

And I don't actually dislike the current guard form, I've been reading
it like:

  guard<mutex>(&my_mutex);

But that is arguably because I've done a fair few years of C++ systems
programming before I got involved with this kernel thing. Also, we use a
very similar syntax for the static_call thing:

  static_call(x86_pmu_enable)(event);


That said; if we were to do this, then something like:

#define __cond_guard(_name, _inst, _fail, args...) \
	CLASS(_name, _inst)(args); \
	if (!__guard_ptr(_name)(&_inst)) _fail

#define cond_guard(_name, _fail, args...) \
	__cond_guard(_name, __UNIQUE_ID(guard), _fail, args)


  cond_guard(spinlock_try, return -EBUSY, &my_lock);


Becomes possible.

Linus, do you like that enough to suffer a flag day patch as proposed by
Oleg?