linux-kernel - Re: [PATCH v3 2/6] cxl/region: Fix a checkpatch warning

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]

Message-ID: <20231011212301.000027e2@Huawei.com>
Date:   Wed, 11 Oct 2023 21:23:01 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Jeongtae Park <jtp.park@...sung.com>
CC:     Dan Williams <dan.j.williams@...el.com>,
        Alison Schofield <alison.schofield@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        "Ben Widawsky" <bwidawsk@...nel.org>,
        Dave Jiang <dave.jiang@...el.com>,
        Davidlohr Bueso <dave@...olabs.net>,
        Fan Ni <fan.ni@...sung.com>, <linux-cxl@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kyungsan Kim <ks0204.kim@...sung.com>,
        "Wonjae Lee" <wj28.lee@...sung.com>,
        Hojin Nam <hj96.nam@...sung.com>,
        Junhyeok Im <junhyeok.im@...sung.com>,
        Jehoon Park <jehoon.park@...sung.com>,
        "Jeongtae Park" <jeongtae.park@...il.com>
Subject: Re: [PATCH v3 2/6] cxl/region: Fix a checkpatch warning

On Tue, 10 Oct 2023 17:26:04 +0900
Jeongtae Park <jtp.park@...sung.com> wrote:

> WARNING: else is not generally useful after a break or return
> 
> Since cpu_cache_invalidate_memregion() already checks for
> support of invalidaton operation, it can be removed.
> This change would make more efficient or small codes
> when 'CONFIG_CXL_REGION_INVALIDATION_TEST' is not set.

Oddly short line wrap - aim for 75 chars ish.

> 
> Signed-off-by: Jeongtae Park <jtp.park@...sung.com>


> ---
>  drivers/cxl/core/region.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e115ba382e04..0eb7a12badb9 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -127,21 +127,15 @@ static struct cxl_region_ref *cxl_rr_load(struct cxl_port *port,
>  
>  static int cxl_region_invalidate_memregion(struct cxl_region *cxlr)
>  {
> -	if (!cpu_cache_has_invalidate_memregion()) {
> -		if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST)) {
> -			dev_warn_once(
> -				&cxlr->dev,
> -				"Bypassing cpu_cache_invalidate_memregion() for testing!\n");
> -			return 0;
> -		} else {
> -			dev_err(&cxlr->dev,
> -				"Failed to synchronize CPU cache state\n");
> -			return -ENXIO;
> -		}
> +	if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST)
> +			&& cpu_cache_has_invalidate_memregion()) {
> +		dev_warn_once(
> +			&cxlr->dev,
> +			"Bypassing cpu_cache_invalidate_memregion() for testing!\n");
> +		return 0;
>  	}
>  
> -	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
> -	return 0;
> +	return cpu_cache_invalidate_memregion(IORES_DESC_CXL);

This is an arch specific call.  Whilst today on x86 (only option) the only way to
return an error is if it's not supported, that's not necessarily going to be the
case for other architectures. So I'd prefer to keep the dev_err if this fails.

Jonathan

>  }
>  
>  static int cxl_region_decode_reset(struct cxl_region *cxlr, int count)