linux-kernel - Re: [PATCH 3/3] cxl/region: Use cond_guard() in show

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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.