Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Fri, 3 Nov 2023 10:30:23 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Oleg Nesterov <oleg@redhat.com>
Cc:     Linus Torvalds <torvalds@linuxfoundation.org>, x86@kernel.org,
        linux-kernel@vger.kernel.org, Jonathan Cameron <jic23@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Eric Biederman <ebiederm@xmission.com>
Subject: Re: [PATCH 1/2] cleanup: Add conditional guard support
Message-ID: <20231103093023.GA8262@noisy.programming.kicks-ass.net>
References: <20231102104429.025835330@infradead.org>
 <20231102110706.460851167@infradead.org>
 <20231102144009.GA9680@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231102144009.GA9680@redhat.com>
Precedence: bulk

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?