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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=whBVhnh=KSeBBRet=E7qJAwnPR_aj5em187Q3FiD+LXnA@mail.gmail.com>
Date: Tue, 27 Feb 2024 12:55:06 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: 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()

On Tue, 27 Feb 2024 at 08:49, Dan Williams <dan.j.williams@...el.com> wrote:
>
> -       rc = down_read_interruptible(&cxl_region_rwsem);
> -       if (rc)
> -               return rc;
> +       cond_guard(rwsem_read_intr, return -EINTR, &cxl_region_rwsem);

Yeah, this is an example of how NOT to do things.

If you can't make the syntax be something clean and sane like

        if (!cond_guard(rwsem_read_intr, &cxl_region_rwsem))
                return -EINTR;

then this code should simply not be converted to guards AT ALL.

Note that we have a perfectly fine way to do conditional lock guarding
by simply using helper functions, which actually makes code MORE
READABLE:

        if (!down_read_interruptible(&cxl_region_rwsem))
                return -EINTR;
        rc = do_locked_function();
        up_read(&cxl_region_rwsem);
        return rc;

and notice how there are no special cases, no multiple unlocks, no
NOTHING. And the syntax is clean.

Honestly, if people are going to use 'guard' to write crap code, we
need to really stop that in its tracks.

There is no upside to making up new interfaces that only generate garbage.

This is final. I'm not willing to even entertain this kind of crap.

                     Linus