From: Youling Tang <[email protected]>
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 <[email protected]>
---
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
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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
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/[email protected]/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/[email protected]/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 <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> 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.
On 29.05.24 г. 6:20 ч., Youling Tang wrote:
> From: Youling Tang <[email protected]>
>
> 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 <[email protected]>
>
> Signed-off-by: Youling Tang <[email protected]>
> ---
> 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
Hi, Nikolay
On 30/05/2024 23:26, Nikolay Borisov wrote:
>
>
> On 29.05.24 г. 6:20 ч., Youling Tang wrote:
>> From: Youling Tang <[email protected]>
>>
>> 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 <[email protected]>
>
>
>>
>> Signed-off-by: Youling Tang <[email protected]>
>> ---
>> 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