mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jemmy Wong <jemmywong512@gmail.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Jemmy <jemmywong512@gmail.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Christian Brauner" <brauner@kernel.org>,
	"Jonathan Cameron" <Jonathan.Cameron@huawei.com>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"David Lechner" <dlechner@baylibre.com>,
	"Przemek Kitszel" <przemyslaw.kitszel@intel.com>,
	"Ingo Molnar" <mingo@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v0] [RFC] cleanup: Unify DEFINE_LOCK_GUARD_0 and DEFINE_LOCK_GUARD_1
Date: Tue, 17 Jun 2025 16:13:26 +0800	[thread overview]
Message-ID: <4513F5D8-A3DF-4477-B54D-D9D614DDC2AF@gmail.com> (raw)
In-Reply-To: <6850686f3088c_2491100fa@dwillia2-xfh.jf.intel.com.notmuch>

Hi Dan,

> On Jun 17, 2025, at 2:54 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> 
> Jemmy Wong wrote:
>> Hi,
>> 
>> This patch consolidates the DEFINE_LOCK_GUARD_0 and DEFINE_LOCK_GUARD_1
>> macros into a single, unified 'DEFINE_LOCK_GUARD' macro to provide
>> a consistent and simplified API for lock guard definitions.
>> 
>> API changes:
>> From    DEFINE_LOCK_GUARD_0(name, lock, unlock, ...)
>> to      DEFINE_LOCK_GUARD(name, *void*, lock, unlock, ...)
>> 
>> From    DEFINE_LOCK_GUARD_1(name, type, lock, unlock, ...)
>> to      DEFINE_LOCK_GUARD(name, type, lock, unlock, ...)
>> 
>> From    CLASS(name, var)(args...)
>> to      CLASS(name, var, args...)
>> 
>> From    guard(name)(args)
>> to      guard(name, args)
> 
> No, I think this organization is instructive for understanding how these
> helpers work. I.e.  that the macro is instantiating a function with an
> automatic variable result, and the arguments to that function arrive in
> @args. This becomes even more important to understand with the ACQUIRE()
> and ACQUIRE_ERR() proposal that instantiate different functions to
> retrieve other properties of the automatic variable result.
> 
>> No change:
>> scoped_guard(name, args...)
>> scoped_cond_guard(name, fail, args...)
> 
> Effectively these are not returning an automatic variable result to the
> current scope and the different calling convention is consistent with
> that difference.

I have some concerns about this point.

Both guard and scoped_guard use CLASS(...) to instantiate an anonymous automatic variable, 
differing only in their scope, while CLASS instantiates a named automatic variable.

To ensure consistency, I believe guard, scoped_guard and CLASS should share the same calling convention.
This change would unify their syntax to: 
CLASS(name, var, args...), guard(name, args...) and scoped_guard(name, args...), 
improving consistency and clarity.

In most cases, an anonymous automatic guard variable suffices to manage object lifecycles, 
aligning with object-oriented programming languages that inherently support automatic lifecycle management 
(constructing objects at initialization and destructing them upon scope exit). 

For example, anonymous object instantiation in other languages includes:
C++: 		People("Alice", 30)
Python: 	People("Alice", 30)
C#: 		new People("Alice", 30)
PHP: 		new People("Alice", 30)
Jave: 		new People("Alice", 30)
JaveScript:	new People("Alice", 30)

This unified syntax would make the API more intuitive and consistent with established programming practices.

>> ---
>> 
>> Deailted changes:
>> 
>> - DEFINE_CLASS(_name, _type, _exit, _init, _init_args...)
>> The void type for _init_args is not required when the constructor takes no arguments,
>> as an int argc is implicitly inserted as the first argument. (int argc, void) is an error.
>> 
>> This patch includes only the core changes.
>> Follow-up patches will be submitted once the approach is accepted.
> 
> Appreciate the RFC first to avoid the thrash while deciding on the
> format change, but it is a nak from me.

Best,
Jemmy



      reply	other threads:[~2025-06-17  8:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-16 11:14 Jemmy Wong
2025-06-16 18:54 ` Dan Williams
2025-06-17  8:13   ` Jemmy Wong [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4513F5D8-A3DF-4477-B54D-D9D614DDC2AF@gmail.com \
    --to=jemmywong512@gmail.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=brauner@kernel.org \
    --cc=dan.j.williams@intel.com \
    --cc=dlechner@baylibre.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=tglx@linutronix.de \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®