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

From: Linus Torvalds <torvalds@linux-foundation.org>
To: 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 12:55:06 -0800	[thread overview]
Message-ID: <CAHk-=whBVhnh=KSeBBRet=E7qJAwnPR_aj5em187Q3FiD+LXnA@mail.gmail.com> (raw)
In-Reply-To: <170905254443.2268463.935306988251313983.stgit@dwillia2-xfh.jf.intel.com>

On Tue, 27 Feb 2024 at 08:49, Dan Williams <dan.j.williams@intel.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