Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Jul 2023 10:27:33 +0100
From:   Conor Dooley <conor.dooley@microchip.com>
To:     wang_yang <wang_yang6662023@163.com>
CC:     <palmer@dabbelt.com>, <aou@eecs.berkeley.edu>,
        <linux-riscv@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
        Wang Yang <yangwang@nj.iscas.ac.cn>
Subject: Re: [PATCH] RISC-V: Use GCC __builtin_prefetch() to implement
 prefetch()
Message-ID: <20230726-washing-scoreless-12f0cca83365@wendy>
References: <20230726074732.32981-1-wang_yang6662023@163.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="7z2nzsGN1jDIioBs"
Content-Disposition: inline
In-Reply-To: <20230726074732.32981-1-wang_yang6662023@163.com>
Precedence: bulk

--7z2nzsGN1jDIioBs
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hey,

On Wed, Jul 26, 2023 at 03:47:32PM +0800, wang_yang wrote:
>  GCC's __builtin_prefetch() was introduced a long time ago, all supported=
 GCC
>  versions have it.So this patch is to use it for implementing the prefetc=
h.
>=20
>  RISC-V Cache Management Operation instructions has been supported by GCC=
 last
>  year.you can refer to
>  https://github.com/gcc-mirror/gcc/commit/3df3ca9014f94fe4af07444fea19b4a=
b29ba8e73

This should be in a Link: tag.

>  It is worth noting that CPU based on RISC-V should support Zicbop extens=
ion.

How do you intend determining whether the CPU supports Zicbop?

>  This has been already done on other architectures (see the commit:
>  https://github.com/torvalds/linux/commit/0453fb3c528c5eb3483441a466b24a4=
cb409eec5).

That's not how to describe commits, please see how that is done
elsewhere in the git history.

>=20
> Signed-off-by: Wang Yang <yangwang@nj.iscas.ac.cn>

WARNING: Possible unwrapped commit description (prefer a maximum 75 chars p=
er line)
#6:=20
 GCC's __builtin_prefetch() was introduced a long time ago, all supported G=
CC

ERROR: Missing Signed-off-by: line by nominal patch author 'wang_yang <wang=
_yang6662023@163.com>'

> ---
>  arch/riscv/include/asm/processor.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>=20
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/=
processor.h
> index c950a8d9edef..f16d4c85ca5b 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -70,6 +70,17 @@ extern void start_thread(struct pt_regs *regs,
> =20
>  extern unsigned long __get_wchan(struct task_struct *p);
> =20
> +#define ARCH_HAS_PREFETCH
> +static inline void prefetch(const void *ptr)
> +{
> +	__builtin_prefetch(ptr, 0, 3);

As far as I can tell, this does not do anything and will be removed by
the compiler. Please take a look at how other extensions are implemented.

Also, why 3? (Your answer should be in the commit message)

Thanks,
Conor.

> +}
> +
> +#define ARCH_HAS_PREFETCHW
> +static inline void prefetchw(const void *ptr)
> +{
> +	__builtin_prefetch(ptr, 1, 3);
> +}
> =20
>  static inline void wait_for_interrupt(void)
>  {
> --=20
> 2.34.1
>=20

--7z2nzsGN1jDIioBs
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZMDnBQAKCRB4tDGHoIJi
0im4AP4zuaPEjwNldgorLZ7XQnAcFueQvdQrx0h7e6ZAN6oYzwD/eO+uxgIz3j5r
iG/Dye9EddOEg2EZ5jMAWCkWn+jHFgk=
=kzQ1
-----END PGP SIGNATURE-----

--7z2nzsGN1jDIioBs--