Messages in this thread | | | Date | Fri, 3 Nov 2023 10:30:23 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH 1/2] cleanup: Add conditional guard support |
| |
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?
| |