Re: [PATCH v2] arm64/mm: Hoist synchronization out of set

public inbox for llvm@lists.linux.dev
 help / color / mirror / Atom feed

* Re: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
       [not found] <20231005140730.2191134-1-ryan.roberts@arm.com>
@ 2023-10-13 21:15 ` kernel test robot
  2023-10-16 17:54   ` Catalin Marinas
  0 siblings, 1 reply; 6+ messages in thread
From: kernel test robot @ 2023-10-13 21:15 UTC (permalink / raw)
  To: Ryan Roberts, Catalin Marinas, Will Deacon, Steven Price,
	Peter Collingbourne
  Cc: llvm, oe-kbuild-all, Ryan Roberts, linux-arm-kernel, linux-kernel

Hi Ryan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on arm-perf/for-next/perf arm/for-next kvmarm/next soc/for-next linus/master v6.6-rc5 next-20231013]
[cannot apply to arm/fixes]
[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/Ryan-Roberts/arm64-mm-Hoist-synchronization-out-of-set_ptes-loop/20231005-231636
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
patch link:    https://lore.kernel.org/r/20231005140730.2191134-1-ryan.roberts%40arm.com
patch subject: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-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/202310140531.BQQwt3NQ-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from net/ipv4/route.c:66:
   In file included from include/linux/mm.h:29:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   1 warning generated.
--
   In file included from sound/soc/qcom/qdsp6/q6apm-dai.c:9:
   In file included from include/sound/soc.h:18:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/arm64/include/asm/hardirq.h:17:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/arm64/include/asm/io.h:12:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   sound/soc/qcom/qdsp6/q6apm-dai.c:355:38: warning: cast from 'void (*)(uint32_t, uint32_t, uint32_t *, void *)' (aka 'void (*)(unsigned int, unsigned int, unsigned int *, void *)') to 'q6apm_cb' (aka 'void (*)(unsigned int, unsigned int, void *, void *)') converts to incompatible function type [-Wcast-function-type-strict]
     355 |         prtd->graph = q6apm_graph_open(dev, (q6apm_cb)event_handler, prtd, graph_id);
         |                                             ^~~~~~~~~~~~~~~~~~~~~~~
   sound/soc/qcom/qdsp6/q6apm-dai.c:499:38: warning: cast from 'void (*)(uint32_t, uint32_t, uint32_t *, void *)' (aka 'void (*)(unsigned int, unsigned int, unsigned int *, void *)') to 'q6apm_cb' (aka 'void (*)(unsigned int, unsigned int, void *, void *)') converts to incompatible function type [-Wcast-function-type-strict]
     499 |         prtd->graph = q6apm_graph_open(dev, (q6apm_cb)event_handler_compr, prtd, graph_id);
         |                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.
--
   In file included from arch/arm64/kernel/mte.c:9:
   In file included from include/linux/mm.h:29:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   arch/arm64/kernel/mte.c:79:20: warning: unused function '__mte_enable_kernel' [-Wunused-function]
      79 | static inline void __mte_enable_kernel(const char *mode, unsigned long tcf)
         |                    ^
   2 warnings generated.
--
   In file included from drivers/gpu/drm/radeon/radeon_ttm.c:33:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:8:
   In file included from include/linux/mm.h:29:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   drivers/gpu/drm/radeon/radeon_ttm.c:200:20: warning: variable 'rbo' set but not used [-Wunused-but-set-variable]
     200 |         struct radeon_bo *rbo;
         |                           ^
   2 warnings generated.
--
   In file included from drivers/gpu/drm/kmb/kmb_dsi.c:12:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/arm64/include/asm/io.h:12:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   drivers/gpu/drm/kmb/kmb_dsi.c:822:2: warning: unused function 'set_test_mode_src_osc_freq_target_low_bits' [-Wunused-function]
     822 |         set_test_mode_src_osc_freq_target_low_bits(struct kmb_dsi *kmb_dsi,
         |         ^
   drivers/gpu/drm/kmb/kmb_dsi.c:834:2: warning: unused function 'set_test_mode_src_osc_freq_target_hi_bits' [-Wunused-function]
     834 |         set_test_mode_src_osc_freq_target_hi_bits(struct kmb_dsi *kmb_dsi,
         |         ^
   3 warnings generated.
--
   In file included from drivers/gpu/drm/nouveau/nouveau_svm.c:22:
   In file included from drivers/gpu/drm/nouveau/nouveau_svm.h:3:
   In file included from drivers/gpu/drm/nouveau/include/nvif/os.h:8:
   In file included from include/linux/pci.h:38:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/arm64/include/asm/hardirq.h:17:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/arm64/include/asm/io.h:12:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   drivers/gpu/drm/nouveau/nouveau_svm.c:115:24: warning: variable 'priority' set but not used [-Wunused-but-set-variable]
     115 |         unsigned target, cmd, priority;
         |                               ^
   drivers/gpu/drm/nouveau/nouveau_svm.c:929:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
     929 |         int ret;
         |             ^
   3 warnings generated.
--
   In file included from drivers/gpu/drm/nouveau/nouveau_connector.c:30:
   In file included from include/linux/vga_switcheroo.h:34:
   In file included from include/linux/fb.h:6:
   In file included from include/linux/kgdb.h:19:
   In file included from include/linux/kprobes.h:28:
   In file included from include/linux/ftrace.h:10:
   In file included from include/linux/trace_recursion.h:5:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/arm64/include/asm/hardirq.h:17:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/arm64/include/asm/io.h:12:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   drivers/gpu/drm/nouveau/nouveau_connector.c:1301:7: warning: variable 'entry' set but not used [-Wunused-but-set-variable]
    1301 |                 u32 entry = ROM16(nv_connector->dcb[0]);
         |                     ^
   2 warnings generated.
--
   In file included from drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c:22:
   In file included from drivers/gpu/drm/nouveau/nvkm/subdev/acr/priv.h:3:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/subdev/acr.h:5:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h:4:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/core/device.h:4:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/core/oclass.h:3:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/core/os.h:4:
   In file included from drivers/gpu/drm/nouveau/include/nvif/os.h:8:
   In file included from include/linux/pci.h:38:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/arm64/include/asm/hardirq.h:17:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/arm64/include/asm/io.h:12:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c:221:7: warning: variable 'loc' set but not used [-Wunused-but-set-variable]
     221 |                 u32 loc, sig, cnt, *meta;
         |                     ^
   2 warnings generated.
--
   In file included from drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c:24:
   In file included from drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h:5:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/subdev/bios.h:4:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h:4:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/core/device.h:4:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/core/oclass.h:3:
   In file included from drivers/gpu/drm/nouveau/include/nvkm/core/os.h:4:
   In file included from drivers/gpu/drm/nouveau/include/nvif/os.h:8:
   In file included from include/linux/pci.h:38:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/arm64/include/asm/hardirq.h:17:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/arm64/include/asm/io.h:12:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c:161:10: warning: cast from 'void (*)(const struct firmware *)' to 'void (*)(void *)' converts to incompatible function type [-Wcast-function-type-strict]
     161 |         .fini = (void(*)(void *))release_firmware,
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   2 warnings generated.
--
   In file included from drivers/gpu/drm/renesas/rcar-du/rcar_cmm.c:8:
   In file included from include/linux/io.h:13:
   In file included from arch/arm64/include/asm/io.h:12:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   drivers/gpu/drm/renesas/rcar-du/rcar_cmm.c:35:19: warning: unused function 'rcar_cmm_read' [-Wunused-function]
      35 | static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
         |                   ^
   2 warnings generated.
--
   In file included from drivers/gpu/drm/qxl/qxl_cmd.c:30:
   In file included from include/drm/drm_util.h:35:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/arm64/include/asm/hardirq.h:17:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/arm64/include/asm/io.h:12:
   In file included from include/linux/pgtable.h:6:
>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
     344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
         |                                                                 ^
   drivers/gpu/drm/qxl/qxl_cmd.c:424:6: warning: variable 'count' set but not used [-Wunused-but-set-variable]
     424 |         int count = 0;
         |             ^
   2 warnings generated.
..


vim +/addr +344 arch/arm64/include/asm/pgtable.h

4f04d8f0054511 Catalin Marinas         2012-03-05  343  
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02 @344) static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  345) 			      pte_t *ptep, pte_t pte, unsigned int nr)
42b2547137f5c9 Kefeng Wang             2022-05-12  346  {
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  347) 	page_table_check_ptes_set(mm, ptep, pte, nr);
3ba82bb647345d Ryan Roberts            2023-10-05  348  	__sync_cache_and_tags(pte, nr);
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  349) 
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  350) 	for (;;) {
3ba82bb647345d Ryan Roberts            2023-10-05  351  		__check_safe_pte_update(mm, ptep, pte);
3ba82bb647345d Ryan Roberts            2023-10-05  352  		set_pte(ptep, pte);
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  353) 		if (--nr == 0)
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  354) 			break;
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  355) 		ptep++;
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  356) 		addr += PAGE_SIZE;
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  357) 		pte_val(pte) += PAGE_SIZE;
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  358) 	}
42b2547137f5c9 Kefeng Wang             2022-05-12  359  }
4a169d61c2ede9 Matthew Wilcox (Oracle  2023-08-02  360) #define set_ptes set_ptes
42b2547137f5c9 Kefeng Wang             2022-05-12  361  

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
  2023-10-13 21:15 ` [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop kernel test robot
@ 2023-10-16 17:54   ` Catalin Marinas
  2023-10-17  7:36     ` Ryan Roberts
  0 siblings, 1 reply; 6+ messages in thread
From: Catalin Marinas @ 2023-10-16 17:54 UTC (permalink / raw)
  To: kernel test robot
  Cc: Ryan Roberts, Will Deacon, Steven Price, Peter Collingbourne,
	llvm, oe-kbuild-all, linux-arm-kernel, linux-kernel

On Sat, Oct 14, 2023 at 05:15:51AM +0800, kernel test robot wrote:
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on arm64/for-next/core]
> [also build test WARNING on arm-perf/for-next/perf arm/for-next kvmarm/next soc/for-next linus/master v6.6-rc5 next-20231013]
> [cannot apply to arm/fixes]
> [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/Ryan-Roberts/arm64-mm-Hoist-synchronization-out-of-set_ptes-loop/20231005-231636
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> patch link:    https://lore.kernel.org/r/20231005140730.2191134-1-ryan.roberts%40arm.com
> patch subject: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
> config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-lkp@intel.com/config)
> compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-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/202310140531.BQQwt3NQ-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from net/ipv4/route.c:66:
>    In file included from include/linux/mm.h:29:
>    In file included from include/linux/pgtable.h:6:
> >> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
>      344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
>          |                                                                 ^
>    1 warning generated.

Thanks for the report. I think something like below will do (I'll test
and commit as a separate patch, it's not something that Ryan's patch
introduces):

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 68984ba9ce2a..b19a8aee684c 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -341,8 +341,9 @@ static inline void __sync_cache_and_tags(pte_t pte, unsigned int nr_pages)
 		mte_sync_tags(pte, nr_pages);
 }
 
-static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
-			      pte_t *ptep, pte_t pte, unsigned int nr)
+static inline void set_ptes(struct mm_struct *mm,
+			    unsigned long __always_unused addr,
+			    pte_t *ptep, pte_t pte, unsigned int nr)
 {
 	page_table_check_ptes_set(mm, ptep, pte, nr);
 	__sync_cache_and_tags(pte, nr);
@@ -353,7 +354,6 @@ static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
 		if (--nr == 0)
 			break;
 		ptep++;
-		addr += PAGE_SIZE;
 		pte_val(pte) += PAGE_SIZE;
 	}
 }
@@ -528,7 +528,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
 #define pud_pfn(pud)		((__pud_to_phys(pud) & PUD_MASK) >> PAGE_SHIFT)
 #define pfn_pud(pfn,prot)	__pud(__phys_to_pud_val((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot))
 
-static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
+static inline void __set_pte_at(struct mm_struct *mm,
+				unsigned long __always_unused addr,
 				pte_t *ptep, pte_t pte, unsigned int nr)
 {
 	__sync_cache_and_tags(pte, nr);

-- 
Catalin

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
  2023-10-16 17:54   ` Catalin Marinas
@ 2023-10-17  7:36     ` Ryan Roberts
  2023-10-17 12:57       ` Catalin Marinas
  0 siblings, 1 reply; 6+ messages in thread
From: Ryan Roberts @ 2023-10-17  7:36 UTC (permalink / raw)
  To: Catalin Marinas, kernel test robot
  Cc: Will Deacon, Steven Price, Peter Collingbourne, llvm,
	oe-kbuild-all, linux-arm-kernel, linux-kernel

On 16/10/2023 18:54, Catalin Marinas wrote:
> On Sat, Oct 14, 2023 at 05:15:51AM +0800, kernel test robot wrote:
>> kernel test robot noticed the following build warnings:
>>
>> [auto build test WARNING on arm64/for-next/core]
>> [also build test WARNING on arm-perf/for-next/perf arm/for-next kvmarm/next soc/for-next linus/master v6.6-rc5 next-20231013]
>> [cannot apply to arm/fixes]
>> [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/Ryan-Roberts/arm64-mm-Hoist-synchronization-out-of-set_ptes-loop/20231005-231636
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
>> patch link:    https://lore.kernel.org/r/20231005140730.2191134-1-ryan.roberts%40arm.com
>> patch subject: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
>> config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-lkp@intel.com/config)
>> compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-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/202310140531.BQQwt3NQ-lkp@intel.com/
>>
>> All warnings (new ones prefixed by >>):
>>
>>    In file included from net/ipv4/route.c:66:
>>    In file included from include/linux/mm.h:29:
>>    In file included from include/linux/pgtable.h:6:
>>>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
>>      344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
>>          |                                                                 ^
>>    1 warning generated.
> 
> Thanks for the report. I think something like below will do (I'll test
> and commit as a separate patch, it's not something that Ryan's patch
> introduces):

I was actually just trying to repro this and was planning to send out a v3 of my
patch. But if you are happy to handle it as you suggest, then I guess you don't
need anything further from me?

Thanks,
Ryan


> 
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 68984ba9ce2a..b19a8aee684c 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -341,8 +341,9 @@ static inline void __sync_cache_and_tags(pte_t pte, unsigned int nr_pages)
>  		mte_sync_tags(pte, nr_pages);
>  }
>  
> -static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
> -			      pte_t *ptep, pte_t pte, unsigned int nr)
> +static inline void set_ptes(struct mm_struct *mm,
> +			    unsigned long __always_unused addr,
> +			    pte_t *ptep, pte_t pte, unsigned int nr)
>  {
>  	page_table_check_ptes_set(mm, ptep, pte, nr);
>  	__sync_cache_and_tags(pte, nr);
> @@ -353,7 +354,6 @@ static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
>  		if (--nr == 0)
>  			break;
>  		ptep++;
> -		addr += PAGE_SIZE;
>  		pte_val(pte) += PAGE_SIZE;
>  	}
>  }
> @@ -528,7 +528,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
>  #define pud_pfn(pud)		((__pud_to_phys(pud) & PUD_MASK) >> PAGE_SHIFT)
>  #define pfn_pud(pfn,prot)	__pud(__phys_to_pud_val((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot))
>  
> -static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
> +static inline void __set_pte_at(struct mm_struct *mm,
> +				unsigned long __always_unused addr,
>  				pte_t *ptep, pte_t pte, unsigned int nr)
>  {
>  	__sync_cache_and_tags(pte, nr);
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
  2023-10-17  7:36     ` Ryan Roberts
@ 2023-10-17 12:57       ` Catalin Marinas
  2023-10-18  8:21         ` Ryan Roberts
  0 siblings, 1 reply; 6+ messages in thread
From: Catalin Marinas @ 2023-10-17 12:57 UTC (permalink / raw)
  To: Ryan Roberts
  Cc: kernel test robot, Will Deacon, Steven Price, Peter Collingbourne,
	llvm, oe-kbuild-all, linux-arm-kernel, linux-kernel

On Tue, Oct 17, 2023 at 08:36:43AM +0100, Ryan Roberts wrote:
> On 16/10/2023 18:54, Catalin Marinas wrote:
> > On Sat, Oct 14, 2023 at 05:15:51AM +0800, kernel test robot wrote:
> >> kernel test robot noticed the following build warnings:
> >>
> >> [auto build test WARNING on arm64/for-next/core]
> >> [also build test WARNING on arm-perf/for-next/perf arm/for-next kvmarm/next soc/for-next linus/master v6.6-rc5 next-20231013]
> >> [cannot apply to arm/fixes]
> >> [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/Ryan-Roberts/arm64-mm-Hoist-synchronization-out-of-set_ptes-loop/20231005-231636
> >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> >> patch link:    https://lore.kernel.org/r/20231005140730.2191134-1-ryan.roberts%40arm.com
> >> patch subject: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
> >> config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-lkp@intel.com/config)
> >> compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
> >> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-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/202310140531.BQQwt3NQ-lkp@intel.com/
> >>
> >> All warnings (new ones prefixed by >>):
> >>
> >>    In file included from net/ipv4/route.c:66:
> >>    In file included from include/linux/mm.h:29:
> >>    In file included from include/linux/pgtable.h:6:
> >>>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
> >>      344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
> >>          |                                                                 ^
> >>    1 warning generated.
> > 
> > Thanks for the report. I think something like below will do (I'll test
> > and commit as a separate patch, it's not something that Ryan's patch
> > introduces):
> 
> I was actually just trying to repro this and was planning to send out a v3 of my
> patch. But if you are happy to handle it as you suggest, then I guess you don't
> need anything further from me?

If you feel like testing, please give this a go ;)

------------8<---------------------------
From e6255237acfc21e92252653c3ed42446ef67f625 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Tue, 17 Oct 2023 11:57:55 +0100
Subject: [PATCH] arm64: Mark the 'addr' argument to set_ptes() and
 __set_pte_at() as unused

This argument is not used by the arm64 implementation. Mark it as
__always_unused and also remove the unnecessary 'addr' increment in
set_ptes().

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202310140531.BQQwt3NQ-lkp@intel.com/
Cc: Will Deacon <will@kernel.org>
Cc: Ryan Roberts <ryan.roberts@arm.com>
---
 arch/arm64/include/asm/pgtable.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 68984ba9ce2a..b19a8aee684c 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -341,8 +341,9 @@ static inline void __sync_cache_and_tags(pte_t pte, unsigned int nr_pages)
 		mte_sync_tags(pte, nr_pages);
 }
 
-static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
-			      pte_t *ptep, pte_t pte, unsigned int nr)
+static inline void set_ptes(struct mm_struct *mm,
+			    unsigned long __always_unused addr,
+			    pte_t *ptep, pte_t pte, unsigned int nr)
 {
 	page_table_check_ptes_set(mm, ptep, pte, nr);
 	__sync_cache_and_tags(pte, nr);
@@ -353,7 +354,6 @@ static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
 		if (--nr == 0)
 			break;
 		ptep++;
-		addr += PAGE_SIZE;
 		pte_val(pte) += PAGE_SIZE;
 	}
 }
@@ -528,7 +528,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
 #define pud_pfn(pud)		((__pud_to_phys(pud) & PUD_MASK) >> PAGE_SHIFT)
 #define pfn_pud(pfn,prot)	__pud(__phys_to_pud_val((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot))
 
-static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
+static inline void __set_pte_at(struct mm_struct *mm,
+				unsigned long __always_unused addr,
 				pte_t *ptep, pte_t pte, unsigned int nr)
 {
 	__sync_cache_and_tags(pte, nr);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
  2023-10-17 12:57       ` Catalin Marinas
@ 2023-10-18  8:21         ` Ryan Roberts
  2023-10-18  9:36           ` Catalin Marinas
  0 siblings, 1 reply; 6+ messages in thread
From: Ryan Roberts @ 2023-10-18  8:21 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: kernel test robot, Will Deacon, Steven Price, Peter Collingbourne,
	llvm, oe-kbuild-all, linux-arm-kernel, linux-kernel

On 17/10/2023 13:57, Catalin Marinas wrote:
> On Tue, Oct 17, 2023 at 08:36:43AM +0100, Ryan Roberts wrote:
>> On 16/10/2023 18:54, Catalin Marinas wrote:
>>> On Sat, Oct 14, 2023 at 05:15:51AM +0800, kernel test robot wrote:
>>>> kernel test robot noticed the following build warnings:
>>>>
>>>> [auto build test WARNING on arm64/for-next/core]
>>>> [also build test WARNING on arm-perf/for-next/perf arm/for-next kvmarm/next soc/for-next linus/master v6.6-rc5 next-20231013]
>>>> [cannot apply to arm/fixes]
>>>> [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/Ryan-Roberts/arm64-mm-Hoist-synchronization-out-of-set_ptes-loop/20231005-231636
>>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
>>>> patch link:    https://lore.kernel.org/r/20231005140730.2191134-1-ryan.roberts%40arm.com
>>>> patch subject: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
>>>> config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-lkp@intel.com/config)
>>>> compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
>>>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-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/202310140531.BQQwt3NQ-lkp@intel.com/
>>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>>    In file included from net/ipv4/route.c:66:
>>>>    In file included from include/linux/mm.h:29:
>>>>    In file included from include/linux/pgtable.h:6:
>>>>>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
>>>>      344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
>>>>          |                                                                 ^
>>>>    1 warning generated.
>>>
>>> Thanks for the report. I think something like below will do (I'll test
>>> and commit as a separate patch, it's not something that Ryan's patch
>>> introduces):
>>
>> I was actually just trying to repro this and was planning to send out a v3 of my
>> patch. But if you are happy to handle it as you suggest, then I guess you don't
>> need anything further from me?
> 
> If you feel like testing, please give this a go ;)

Compile tested and observed that warning is gone with your change. Also ran mm
selftests and all looks good. So:

Tested-by: Ryan Roberts <ryan.roberts@arm.com>


> 
> ------------8<---------------------------
> From e6255237acfc21e92252653c3ed42446ef67f625 Mon Sep 17 00:00:00 2001
> From: Catalin Marinas <catalin.marinas@arm.com>
> Date: Tue, 17 Oct 2023 11:57:55 +0100
> Subject: [PATCH] arm64: Mark the 'addr' argument to set_ptes() and
>  __set_pte_at() as unused
> 
> This argument is not used by the arm64 implementation. Mark it as
> __always_unused and also remove the unnecessary 'addr' increment in
> set_ptes().
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202310140531.BQQwt3NQ-lkp@intel.com/
> Cc: Will Deacon <will@kernel.org>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> ---
>  arch/arm64/include/asm/pgtable.h | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 68984ba9ce2a..b19a8aee684c 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -341,8 +341,9 @@ static inline void __sync_cache_and_tags(pte_t pte, unsigned int nr_pages)
>  		mte_sync_tags(pte, nr_pages);
>  }
>  
> -static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
> -			      pte_t *ptep, pte_t pte, unsigned int nr)
> +static inline void set_ptes(struct mm_struct *mm,
> +			    unsigned long __always_unused addr,

Personally I'm not a huge fan of the __always_unused mark up, given that it's
used so patchily in the source. The warning also disappears without these
markups. (the real problem is just the `addr += PAGE_SIZE` below.

Thanks,
Ryan


> +			    pte_t *ptep, pte_t pte, unsigned int nr)
>  {
>  	page_table_check_ptes_set(mm, ptep, pte, nr);
>  	__sync_cache_and_tags(pte, nr);
> @@ -353,7 +354,6 @@ static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
>  		if (--nr == 0)
>  			break;
>  		ptep++;
> -		addr += PAGE_SIZE;
>  		pte_val(pte) += PAGE_SIZE;
>  	}
>  }
> @@ -528,7 +528,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
>  #define pud_pfn(pud)		((__pud_to_phys(pud) & PUD_MASK) >> PAGE_SHIFT)
>  #define pfn_pud(pfn,prot)	__pud(__phys_to_pud_val((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot))
>  
> -static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
> +static inline void __set_pte_at(struct mm_struct *mm,
> +				unsigned long __always_unused addr,
>  				pte_t *ptep, pte_t pte, unsigned int nr)
>  {
>  	__sync_cache_and_tags(pte, nr);


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
  2023-10-18  8:21         ` Ryan Roberts
@ 2023-10-18  9:36           ` Catalin Marinas
  0 siblings, 0 replies; 6+ messages in thread
From: Catalin Marinas @ 2023-10-18  9:36 UTC (permalink / raw)
  To: Ryan Roberts
  Cc: kernel test robot, Will Deacon, Steven Price, Peter Collingbourne,
	llvm, oe-kbuild-all, linux-arm-kernel, linux-kernel

On Wed, Oct 18, 2023 at 09:21:02AM +0100, Ryan Roberts wrote:
> On 17/10/2023 13:57, Catalin Marinas wrote:
> > On Tue, Oct 17, 2023 at 08:36:43AM +0100, Ryan Roberts wrote:
> >> On 16/10/2023 18:54, Catalin Marinas wrote:
> >>> On Sat, Oct 14, 2023 at 05:15:51AM +0800, kernel test robot wrote:
> >>>> kernel test robot noticed the following build warnings:
> >>>>
> >>>> [auto build test WARNING on arm64/for-next/core]
> >>>> [also build test WARNING on arm-perf/for-next/perf arm/for-next kvmarm/next soc/for-next linus/master v6.6-rc5 next-20231013]
> >>>> [cannot apply to arm/fixes]
> >>>> [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/Ryan-Roberts/arm64-mm-Hoist-synchronization-out-of-set_ptes-loop/20231005-231636
> >>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> >>>> patch link:    https://lore.kernel.org/r/20231005140730.2191134-1-ryan.roberts%40arm.com
> >>>> patch subject: [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop
> >>>> config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-lkp@intel.com/config)
> >>>> compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
> >>>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231014/202310140531.BQQwt3NQ-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/202310140531.BQQwt3NQ-lkp@intel.com/
> >>>>
> >>>> All warnings (new ones prefixed by >>):
> >>>>
> >>>>    In file included from net/ipv4/route.c:66:
> >>>>    In file included from include/linux/mm.h:29:
> >>>>    In file included from include/linux/pgtable.h:6:
> >>>>>> arch/arm64/include/asm/pgtable.h:344:65: warning: parameter 'addr' set but not used [-Wunused-but-set-parameter]
> >>>>      344 | static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
> >>>>          |                                                                 ^
> >>>>    1 warning generated.
> >>>
> >>> Thanks for the report. I think something like below will do (I'll test
> >>> and commit as a separate patch, it's not something that Ryan's patch
> >>> introduces):
> >>
> >> I was actually just trying to repro this and was planning to send out a v3 of my
> >> patch. But if you are happy to handle it as you suggest, then I guess you don't
> >> need anything further from me?
> > 
> > If you feel like testing, please give this a go ;)
> 
> Compile tested and observed that warning is gone with your change. Also ran mm
> selftests and all looks good. So:
> 
> Tested-by: Ryan Roberts <ryan.roberts@arm.com>

Thanks. I'll push this patch on top of yours.

-- 
Catalin

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-10-18  9:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20231005140730.2191134-1-ryan.roberts@arm.com>
2023-10-13 21:15 ` [PATCH v2] arm64/mm: Hoist synchronization out of set_ptes() loop kernel test robot
2023-10-16 17:54   ` Catalin Marinas
2023-10-17  7:36     ` Ryan Roberts
2023-10-17 12:57       ` Catalin Marinas
2023-10-18  8:21         ` Ryan Roberts
2023-10-18  9:36           ` Catalin Marinas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox