Re: [PATCH 3/3] cxl/region: Use cond_guard() in show_targetN()

From: Dan Williams
Date: Tue Feb 27 2024 - 18:57:23 EST


Linus Torvalds wrote:
> On Tue, 27 Feb 2024 at 13:42, Dan Williams <dan.j.williams@xxxxxxxxx> 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.