From: Dan Williams <dan.j.williams@intel.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
Dan Williams <dan.j.williams@intel.com>
Cc: <peterz@infradead.org>, <gregkh@linuxfoundation.org>,
Ira Weiny <ira.weiny@intel.com>,
Dave Jiang <dave.jiang@intel.com>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>,
"Fabio M. De Francesco"
<fabio.maria.de.francesco@linux.intel.com>,
<linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>
Subject: Re: [PATCH 3/3] cxl/region: Use cond_guard() in show_targetN()
Date: Tue, 27 Feb 2024 15:56:51 -0800 [thread overview]
Message-ID: <65de76c3be2ad_1138c729419@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <CAHk-=whYxkfLVtBW_B-PgNqhKOAThTbfoH5CxtOTkwOB6VOt6w@mail.gmail.com>
Linus Torvalds wrote:
> On Tue, 27 Feb 2024 at 13:42, Dan Williams <dan.j.williams@intel.com> wrote:
> >
> > I will also note that these last 3 statements, nuking the proposal from
> > space, I find excessive. Yes, on the internet no one can hear you being
> > subtle, but the "MORE READABLE" and "NOTHING" were pretty darn
> > unequivocal, especially coming from the person who has absolute final
> > say on what enters his project.
>
> Heh. It's not just " one can hear you being subtle", sometimes it's
> also "people don't take hints". It can be hard to tell..
I appreciate that. It is difficult to judge what size clue bat to carry
from one thread to the next.
> Anyway, it's not that I hate the guard things in general. But I do
> think they need to be used carefully, and I do think it's very
> important that they have clean interfaces.
>
> The current setup came about after quite long discussions about
> getting reasonable syntax, and I'm still a bit worried even about the
> current simpler ones.
>
> And by "simpler ones" I don't mean our current scoped_cond_guard()
> thing. We have exactly one user of it, and I have considered getting
> rid of that one user because I think it's absolutely horrid. I haven't
> figured out a better syntax for it.
>
> For the non-scoped version, I actually think there *would* be a better
> syntax - putting the error case after the macro (the way we put the
> success case after the macro for the scoped one).
>
> In fact, maybe the solution is to make the scoped and non-scoped
> versions act very similar: we could do something like this:
>
> [scoped_]cond_guard(name, args) { success } else { fail };
>
> and that syntax feels much more C-line to me.
>
> So maybe something like the attached (TOTALLY UNTESTED!!) patch for
> the scoped version, and then the non-scoped version would have the
> same syntax (except it would have to generate that __UNIQUE_ID()
> thing, of course).
This would have definitely saved me from thinking that passing a
"return" statement to a macro was an idea worth copying. I like that it
puts the onus on the caller to understand "this is a conditional" you
are responsible for handling the conditions, the macro is only handling
releasing the lock at the end of the scope".
> I haven't thought much about this. But I think this would be more
> acceptable to me, and also solve some of the ugliness with the current
> pre-existing scoped_cond_guard().
>
> I dunno. PeterZ did the existing stuff, but he's offlined due to
> shoulder problems so not likely to chip in.
Ah, ok, yeah has been quiet on this thread for a while. I will take some
inspiration from this proposal and huddle again with Fabio.
Thanks for the nudge.
next prev parent reply other threads:[~2024-02-27 23:56 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-27 16:48 [PATCH 0/2] cleanup: A couple extensions for conditional resource management Dan Williams
2024-02-27 16:48 ` [PATCH 1/3] cleanup: Add cond_guard() to conditional guards Dan Williams
2024-02-27 20:49 ` Linus Torvalds
2024-02-27 16:48 ` [PATCH 2/3] cleanup: Introduce cond_no_free_ptr() Dan Williams
2024-02-27 20:40 ` Linus Torvalds
2024-02-27 16:49 ` [PATCH 3/3] cxl/region: Use cond_guard() in show_targetN() Dan Williams
2024-02-27 20:55 ` Linus Torvalds
2024-02-27 21:41 ` Dan Williams
2024-02-27 22:34 ` Linus Torvalds
2024-02-27 23:56 ` Dan Williams [this message]
2024-10-01 22:30 [PATCH 0/3] cleanup: add if_not_cond_guard macro David Lechner
2024-10-01 22:30 ` [PATCH 3/3] cxl/region: Use cond_guard() in show_targetN() David Lechner
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=65de76c3be2ad_1138c729419@dwillia2-xfh.jf.intel.com.notmuch \
--to=dan.j.williams@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=dave.jiang@intel.com \
--cc=fabio.maria.de.francesco@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=ira.weiny@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=torvalds@linux-foundation.org \
/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®