[PATCH 1/2] x86: Remove the prefetch() specific implementation on x86_64

[PATCH 1/2] x86: Remove the prefetch() specific implementation on x86_64

[Youling Tang]

From: Youling Tang <tangyouling@kylinos.cn>

After commit ab483570a13b ("x86 & generic: change to __builtin_prefetch()"),
x86_64 directly uses __builtin_prefetch() without the specific implementation
of prefetch(). Also, x86_64 use a generic definition until commit ae2e15eb3b6c
("x86: unify prefetch operations"). So remove it.

Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
 arch/x86/include/asm/processor.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index cb4f6c513c48..44371bdcc59d 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -599,9 +599,6 @@ extern char			ignore_fpu_irq;
 #ifdef CONFIG_X86_32
 # define BASE_PREFETCH		""
 # define ARCH_HAS_PREFETCH
-#else
-# define BASE_PREFETCH		"prefetcht0 %1"
-#endif
 
 /*
  * Prefetch instructions for Pentium III (+) and AMD Athlon (+)
@@ -616,6 +613,10 @@ static inline void prefetch(const void *x)
 			  "m" (*(const char *)x));
 }
 
+#else
+# define BASE_PREFETCH		"prefetcht0 %1"
+#endif
+
 /*
  * 3dnow prefetch to get an exclusive cache line.
  * Useful for spinlocks to avoid one state transition in the
-- 
2.34.1

[PATCH 2/2] prefetch: Add ARCH_HAS_PREFETCH definition when the architecture is not defined

[Youling Tang]

From: Youling Tang <tangyouling@kylinos.cn>

After commit ab483570a13b ("x86 & generic: change to __builtin_prefetch()"), the
__builtin_prefetch implementation will be used by default, so add its definition
to return to a state similar to before commit[1].

Currently prefetch_range() will be empty implemented under the x86_64 architecture,
there was a concrete implementation before "x86 & generic: change to
__builtin_prefetch()", so fix it.

No similar changes have been made to ARCH_HAS_PREFETCHW at this time.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/include/linux/prefetch.h?id=8e4f2fd31bf737abb392e694898a1496157623b5

Fixes: ab483570a13b ("x86 & generic: change to __builtin_prefetch()")
Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
 include/linux/prefetch.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/prefetch.h b/include/linux/prefetch.h
index b068e2e60939..162b7105e37c 100644
--- a/include/linux/prefetch.h
+++ b/include/linux/prefetch.h
@@ -36,6 +36,7 @@ struct page;
 */
 
 #ifndef ARCH_HAS_PREFETCH
+#define ARCH_HAS_PREFETCH
 #define prefetch(x) __builtin_prefetch(x)
 #endif
 
-- 
2.34.1

Re: [PATCH 2/2] prefetch: Add ARCH_HAS_PREFETCH definition when the architecture is not defined

[Nikolay Borisov]

On 29.05.24 г. 6:20 ч., Youling Tang wrote:
> From: Youling Tang <tangyouling@kylinos.cn>
> 
> After commit ab483570a13b ("x86 & generic: change to __builtin_prefetch()"), the
> __builtin_prefetch implementation will be used by default, so add its definition
> to return to a state similar to before commit[1].

I find those references to the past state somewhat confusing and not 
really adding sustenance to the explanation.

> 
> Currently prefetch_range() will be empty implemented under the x86_64 architecture,
> there was a concrete implementation before "x86 & generic: change to
> __builtin_prefetch()", so fix it.

Actually prefetch_range will be empty for every architecture which 
doesn't defined ARCH_HAS_PREFETCH and since we have a working generic 
fallback this indeed seems backwards. So defining ARCH_HAS_PREFETCH 
makes sense.

Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>

> 
> No similar changes have been made to ARCH_HAS_PREFETCHW at this time.
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/include/linux/prefetch.h?id=8e4f2fd31bf737abb392e694898a1496157623b5
> 
> Fixes: ab483570a13b ("x86 & generic: change to __builtin_prefetch()")
> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
> ---
>   include/linux/prefetch.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/prefetch.h b/include/linux/prefetch.h
> index b068e2e60939..162b7105e37c 100644
> --- a/include/linux/prefetch.h
> +++ b/include/linux/prefetch.h
> @@ -36,6 +36,7 @@ struct page;
>   */
>   
>   #ifndef ARCH_HAS_PREFETCH
> +#define ARCH_HAS_PREFETCH
>   #define prefetch(x) __builtin_prefetch(x)
>   #endif
>   

Re: [PATCH 1/2] x86: Remove the prefetch() specific implementation on x86_64

[kernel test robot]

Hi Youling,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/master]
[also build test ERROR on linus/master v6.10-rc1 next-20240529]
[cannot apply to tip/auto-latest tip/x86/core bp/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Youling-Tang/prefetch-Add-ARCH_HAS_PREFETCH-definition-when-the-architecture-is-not-defined/20240529-112345
base:   tip/master
patch link:    https://lore.kernel.org/r/20240529032059.899347-1-youling.tang%40linux.dev
patch subject: [PATCH 1/2] x86: Remove the prefetch() specific implementation on x86_64
config: x86_64-buildonly-randconfig-006-20240530 (https://download.01.org/0day-ci/archive/20240530/202405300328.eZmSYZrP-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240530/202405300328.eZmSYZrP-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405300328.eZmSYZrP-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/soc/fsl/dpio/dpio-service.c: In function 'dpaa2_io_store_next':
>> drivers/soc/fsl/dpio/dpio-service.c:745:17: error: implicit declaration of function 'prefetch'; did you mean 'prefetchw'? [-Werror=implicit-function-declaration]
     745 |                 prefetch(&s->vaddr[s->idx]);
         |                 ^~~~~~~~
         |                 prefetchw
   cc1: some warnings being treated as errors
--
   drivers/soc/fsl/dpio/qbman-portal.c: In function 'qbman_swp_dqrr_next_direct':
>> drivers/soc/fsl/dpio/qbman-portal.c:1213:17: error: implicit declaration of function 'prefetch'; did you mean 'prefetchw'? [-Werror=implicit-function-declaration]
    1213 |                 prefetch(qbman_get_cmd(s,
         |                 ^~~~~~~~
         |                 prefetchw
   cc1: some warnings being treated as errors


vim +745 drivers/soc/fsl/dpio/dpio-service.c

780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  703  
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  704  /**
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  705   * dpaa2_io_store_next() - Determine when the next dequeue result is available.
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  706   * @s: the dpaa2_io_store object.
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  707   * @is_last: indicate whether this is the last frame in the pull command.
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  708   *
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  709   * When an object driver performs dequeues to a dpaa2_io_store, this function
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  710   * can be used to determine when the next frame result is available. Once
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  711   * this function returns non-NULL, a subsequent call to it will try to find
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  712   * the next dequeue result.
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  713   *
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  714   * Note that if a pull-dequeue has a NULL result because the target FQ/channel
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  715   * was empty, then this function will also return NULL (rather than expecting
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  716   * the caller to always check for this. As such, "is_last" can be used to
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  717   * differentiate between "end-of-empty-dequeue" and "still-waiting".
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  718   *
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  719   * Return dequeue result for a valid dequeue result, or NULL for empty dequeue.
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  720   */
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  721  struct dpaa2_dq *dpaa2_io_store_next(struct dpaa2_io_store *s, int *is_last)
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  722  {
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  723  	int match;
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  724  	struct dpaa2_dq *ret = &s->vaddr[s->idx];
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  725  
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  726  	match = qbman_result_has_new_result(s->swp, ret);
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  727  	if (!match) {
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  728  		*is_last = 0;
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  729  		return NULL;
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  730  	}
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  731  
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  732  	s->idx++;
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  733  
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  734  	if (dpaa2_dq_is_pull_complete(ret)) {
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  735  		*is_last = 1;
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  736  		s->idx = 0;
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  737  		/*
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  738  		 * If we get an empty dequeue result to terminate a zero-results
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  739  		 * vdqcr, return NULL to the caller rather than expecting him to
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  740  		 * check non-NULL results every time.
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  741  		 */
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  742  		if (!(dpaa2_dq_flags(ret) & DPAA2_DQ_STAT_VALIDFRAME))
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  743  			ret = NULL;
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  744  	} else {
f1e250bf365962 drivers/soc/fsl/dpio/dpio-service.c            Ioana Ciocoi Radulescu 2018-12-14 @745  		prefetch(&s->vaddr[s->idx]);
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  746  		*is_last = 0;
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  747  	}
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  748  
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  749  	return ret;
780b626323d721 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Roy Pledge             2017-03-13  750  }
53639c64c686f0 drivers/staging/fsl-mc/bus/dpio/dpio-service.c Laurentiu Tudor        2017-11-17  751  EXPORT_SYMBOL_GPL(dpaa2_io_store_next);
e80081c34b0358 drivers/soc/fsl/dpio/dpio-service.c            Roy Pledge             2018-12-18  752  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] x86: Remove the prefetch() specific implementation on x86_64

[Youling Tang]

On 30/05/2024 04:03, kernel test robot wrote:

> Hi Youling,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on tip/master]
> [also build test ERROR on linus/master v6.10-rc1 next-20240529]
> [cannot apply to tip/auto-latest tip/x86/core bp/for-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Youling-Tang/prefetch-Add-ARCH_HAS_PREFETCH-definition-when-the-architecture-is-not-defined/20240529-112345
> base:   tip/master
> patch link:    https://lore.kernel.org/r/20240529032059.899347-1-youling.tang%40linux.dev
> patch subject: [PATCH 1/2] x86: Remove the prefetch() specific implementation on x86_64
> config: x86_64-buildonly-randconfig-006-20240530 (https://download.01.org/0day-ci/archive/20240530/202405300328.eZmSYZrP-lkp@intel.com/config)
> compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240530/202405300328.eZmSYZrP-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202405300328.eZmSYZrP-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>     drivers/soc/fsl/dpio/dpio-service.c: In function 'dpaa2_io_store_next':
>>> drivers/soc/fsl/dpio/dpio-service.c:745:17: error: implicit declaration of function 'prefetch'; did you mean 'prefetchw'? [-Werror=implicit-function-declaration]
>       745 |                 prefetch(&s->vaddr[s->idx]);
>           |                 ^~~~~~~~
>           |                 prefetchw
>     cc1: some warnings being treated as errors
> --
>     drivers/soc/fsl/dpio/qbman-portal.c: In function 'qbman_swp_dqrr_next_direct':
>>> drivers/soc/fsl/dpio/qbman-portal.c:1213:17: error: implicit declaration of function 'prefetch'; did you mean 'prefetchw'? [-Werror=implicit-function-declaration]
>      1213 |                 prefetch(qbman_get_cmd(s,
>           |                 ^~~~~~~~
>           |                 prefetchw
>     cc1: some warnings being treated as errors
This problem is caused by not including the linux/prefetch.h file. There 
were
no build errors earlier because the definitions in processor.h were used
indirectly. (For architectures that do not implement prefetch, this build
error can occur without the patch).

We can fix it in the following way:
diff --git a/drivers/soc/fsl/dpio/dpio-service.c 
b/drivers/soc/fsl/dpio/dpio-service.c
index b811446e0fa5..a4692b9ad8d7 100644
--- a/drivers/soc/fsl/dpio/dpio-service.c
+++ b/drivers/soc/fsl/dpio/dpio-service.c
@@ -9,6 +9,7 @@
  #include <soc/fsl/dpaa2-io.h>
  #include <linux/init.h>
  #include <linux/module.h>
+#include <linux/prefetch.h>
  #include <linux/platform_device.h>
  #include <linux/interrupt.h>
  #include <linux/dma-mapping.h>
diff --git a/drivers/soc/fsl/dpio/qbman-portal.c 
b/drivers/soc/fsl/dpio/qbman-portal.c
index 0a3fb6c115f4..1c0bf04b101c 100644
--- a/drivers/soc/fsl/dpio/qbman-portal.c
+++ b/drivers/soc/fsl/dpio/qbman-portal.c
@@ -7,6 +7,7 @@

  #include <asm/cacheflush.h>
  #include <linux/io.h>
+#include <linux/prefetch.h>
  #include <linux/slab.h>

Thanks,
Youling.

Re: [PATCH 1/2] x86: Remove the prefetch() specific implementation on x86_64

[Nikolay Borisov]

On 29.05.24 г. 6:20 ч., Youling Tang wrote:
> From: Youling Tang <tangyouling@kylinos.cn>
> 
> After commit ab483570a13b ("x86 & generic: change to __builtin_prefetch()"),
> x86_64 directly uses __builtin_prefetch() without the specific implementation
> of prefetch(). Also, x86_64 use a generic definition until commit ae2e15eb3b6c
> ("x86: unify prefetch operations"). So remove it.


So this patch just ensures the x86-specific prefetch() implementation is 
defined only for 32bit case, otherwise we have it defined for the 64bit 
case as well but effectively it's not used since ARCH_HAS_PREFETCH is 
not defined for 64bit, meaning in the 64bit case prefetch() is still 
defined to __builtint_prefetch in include/linux/prefetch.h.


In essence this is a purely cosmetic cleanup , am I right?


I compiled a file that utilizes prefetch with and without your patch and 
the generated assembly is identical.


Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>


> 
> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
> ---
>   arch/x86/include/asm/processor.h | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index cb4f6c513c48..44371bdcc59d 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -599,9 +599,6 @@ extern char			ignore_fpu_irq;
>   #ifdef CONFIG_X86_32
>   # define BASE_PREFETCH		""
>   # define ARCH_HAS_PREFETCH
> -#else
> -# define BASE_PREFETCH		"prefetcht0 %1"
> -#endif
>   
>   /*
>    * Prefetch instructions for Pentium III (+) and AMD Athlon (+)
> @@ -616,6 +613,10 @@ static inline void prefetch(const void *x)
>   			  "m" (*(const char *)x));
>   }
>   
> +#else
> +# define BASE_PREFETCH		"prefetcht0 %1"
> +#endif
> +
>   /*
>    * 3dnow prefetch to get an exclusive cache line.
>    * Useful for spinlocks to avoid one state transition in the

Re: [PATCH 1/2] x86: Remove the prefetch() specific implementation on x86_64

[Youling Tang]

Hi, Nikolay
On 30/05/2024 23:26, Nikolay Borisov wrote:
>
>
> On 29.05.24 г. 6:20 ч., Youling Tang wrote:
>> From: Youling Tang <tangyouling@kylinos.cn>
>>
>> After commit ab483570a13b ("x86 & generic: change to 
>> __builtin_prefetch()"),
>> x86_64 directly uses __builtin_prefetch() without the specific 
>> implementation
>> of prefetch(). Also, x86_64 use a generic definition until commit 
>> ae2e15eb3b6c
>> ("x86: unify prefetch operations"). So remove it.
>
>
> So this patch just ensures the x86-specific prefetch() implementation 
> is defined only for 32bit case, otherwise we have it defined for the 
> 64bit case as well but effectively it's not used since 
> ARCH_HAS_PREFETCH is not defined for 64bit, meaning in the 64bit case 
> prefetch() is still defined to __builtint_prefetch in 
> include/linux/prefetch.h.
>
>
> In essence this is a purely cosmetic cleanup , am I right?
Yes, when arch customization and __builtint_prefetch are implemented with
the same instructions, it looks like pure cleaning (without changing the
generated assembly).

Thanks,
Youling.
>
>
> I compiled a file that utilizes prefetch with and without your patch 
> and the generated assembly is identical.
>
>
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
>
>
>>
>> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
>> ---
>>   arch/x86/include/asm/processor.h | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/processor.h 
>> b/arch/x86/include/asm/processor.h
>> index cb4f6c513c48..44371bdcc59d 100644
>> --- a/arch/x86/include/asm/processor.h
>> +++ b/arch/x86/include/asm/processor.h
>> @@ -599,9 +599,6 @@ extern char            ignore_fpu_irq;
>>   #ifdef CONFIG_X86_32
>>   # define BASE_PREFETCH        ""
>>   # define ARCH_HAS_PREFETCH
>> -#else
>> -# define BASE_PREFETCH        "prefetcht0 %1"
>> -#endif
>>     /*
>>    * Prefetch instructions for Pentium III (+) and AMD Athlon (+)
>> @@ -616,6 +613,10 @@ static inline void prefetch(const void *x)
>>                 "m" (*(const char *)x));
>>   }
>>   +#else
>> +# define BASE_PREFETCH        "prefetcht0 %1"
>> +#endif
>> +
>>   /*
>>    * 3dnow prefetch to get an exclusive cache line.
>>    * Useful for spinlocks to avoid one state transition in the

Re: [PATCH 1/2] x86: Remove the prefetch() specific implementation on x86_64

[Youling Tang]

Hi,

On 30/05/2024 23:26, Nikolay Borisov wrote:
>
>
> On 29.05.24 г. 6:20 ч., Youling Tang wrote:
>> From: Youling Tang <tangyouling@kylinos.cn>
>>
>> After commit ab483570a13b ("x86 & generic: change to 
>> __builtin_prefetch()"),
>> x86_64 directly uses __builtin_prefetch() without the specific 
>> implementation
>> of prefetch(). Also, x86_64 use a generic definition until commit 
>> ae2e15eb3b6c
>> ("x86: unify prefetch operations"). So remove it.
>
>
> So this patch just ensures the x86-specific prefetch() implementation 
> is defined only for 32bit case, otherwise we have it defined for the 
> 64bit case as well but effectively it's not used since 
> ARCH_HAS_PREFETCH is not defined for 64bit, meaning in the 64bit case 
> prefetch() is still defined to __builtint_prefetch in 
> include/linux/prefetch.h.
>
>
> In essence this is a purely cosmetic cleanup , am I right?
>
>
> I compiled a file that utilizes prefetch with and without your patch 
> and the generated assembly is identical.
>
>
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
>
>
>>
>> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
>> ---
>>   arch/x86/include/asm/processor.h | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/processor.h 
>> b/arch/x86/include/asm/processor.h
>> index cb4f6c513c48..44371bdcc59d 100644
>> --- a/arch/x86/include/asm/processor.h
>> +++ b/arch/x86/include/asm/processor.h
>> @@ -599,9 +599,6 @@ extern char            ignore_fpu_irq;
>>   #ifdef CONFIG_X86_32
>>   # define BASE_PREFETCH        ""
>>   # define ARCH_HAS_PREFETCH
>> -#else
>> -# define BASE_PREFETCH        "prefetcht0 %1"
>> -#endif
>>     /*
>>    * Prefetch instructions for Pentium III (+) and AMD Athlon (+)
>> @@ -616,6 +613,10 @@ static inline void prefetch(const void *x)
>>                 "m" (*(const char *)x));
>>   }
>>   +#else
>> +# define BASE_PREFETCH        "prefetcht0 %1"
>> +#endif
>> +
>>   /*
>>    * 3dnow prefetch to get an exclusive cache line.
>>    * Useful for spinlocks to avoid one state transition in the
Sorry to bother you, but do we still need this patchset? (Do I need to
modify the commit message and send v2 if necessary?)

Thanks,
Youling.