[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <65de76c3be2ad_1138c729419@dwillia2-xfh.jf.intel.com.notmuch>
Date: Tue, 27 Feb 2024 15:56:51 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>, Dan Williams
<dan.j.williams@...el.com>
CC: <peterz@...radead.org>, <gregkh@...uxfoundation.org>, Ira Weiny
<ira.weiny@...el.com>, Dave Jiang <dave.jiang@...el.com>, Jonathan Cameron
<Jonathan.Cameron@...wei.com>, "Fabio M. De Francesco"
<fabio.maria.de.francesco@...ux.intel.com>, <linux-kernel@...r.kernel.org>,
<linux-cxl@...r.kernel.org>
Subject: Re: [PATCH 3/3] cxl/region: Use cond_guard() in show_targetN()
Linus Torvalds wrote:
> On Tue, 27 Feb 2024 at 13:42, Dan Williams <dan.j.williams@...el.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.
Powered by blists - more mailing lists