diff mbox series

[V2] mm: Drop unused set_pte_safe()

Message ID 20240910101026.428808-1-anshuman.khandual@arm.com (mailing list archive)
State New
Headers show
Series [V2] mm: Drop unused set_pte_safe() | expand

Commit Message

Anshuman Khandual Sept. 10, 2024, 10:10 a.m. UTC 
All set_pte_safe() usage have been dropped after the commit eccd906484d1
("x86/mm: Do not use set_{pud, pmd}_safe() when splitting a large page")
This just drops now unused helper set_pte_safe().

Besides this macro was buggy due to doing direct dereferencing of the pte,
and if it were to be kept, it should have been updated to use a single call
to ptep_get().

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: "Mike Rapoport (IBM)" <rppt@kernel.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
Changes in V2:

- Updated the commit message per Ryan

Changes in V1:

https://lore.kernel.org/all/20240910090409.374424-1-anshuman.khandual@arm.com/

 include/linux/pgtable.h | 6 ------
 1 file changed, 6 deletions(-)

Comments

Andrew Morton Sept. 10, 2024, 8:13 p.m. UTC | #1 
On Tue, 10 Sep 2024 15:40:26 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:

> All set_pte_safe() usage have been dropped after the commit eccd906484d1
> ("x86/mm: Do not use set_{pud, pmd}_safe() when splitting a large page")
> This just drops now unused helper set_pte_safe().
> 
> Besides this macro was buggy due to doing direct dereferencing of the pte,
> and if it were to be kept, it should have been updated to use a single call
> to ptep_get().
> 

arch/x86/mm/init_64.c: In function 'set_pte_init':
arch/x86/mm/init_64.c:83:17: error: implicit declaration of function 'set_pte_safe'; did you mean 'set_pmd_safe'? [-Werror=implicit-function-declaration]
   83 |                 set_##type1##_safe(arg1, arg2);                 \
      |                 ^~~~
arch/x86/mm/init_64.c:91:1: note: in expansion of macro 'DEFINE_ENTRY'
   91 | DEFINE_ENTRY(pte, pte, init)
      | ^~~~~~~~~~~~
Anshuman Khandual Sept. 11, 2024, 3:52 a.m. UTC | #2 
On 9/11/24 01:43, Andrew Morton wrote:
> On Tue, 10 Sep 2024 15:40:26 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
>> All set_pte_safe() usage have been dropped after the commit eccd906484d1
>> ("x86/mm: Do not use set_{pud, pmd}_safe() when splitting a large page")
>> This just drops now unused helper set_pte_safe().
>>
>> Besides this macro was buggy due to doing direct dereferencing of the pte,
>> and if it were to be kept, it should have been updated to use a single call
>> to ptep_get().
>>
> 
> arch/x86/mm/init_64.c: In function 'set_pte_init':
> arch/x86/mm/init_64.c:83:17: error: implicit declaration of function 'set_pte_safe'; did you mean 'set_pmd_safe'? [-Werror=implicit-function-declaration]
>    83 |                 set_##type1##_safe(arg1, arg2);                 \
>       |                 ^~~~
> arch/x86/mm/init_64.c:91:1: note: in expansion of macro 'DEFINE_ENTRY'
>    91 | DEFINE_ENTRY(pte, pte, init)
>       | ^~~~~~~~~~~~

kernel_physical_mapping_init()
	__kernel_physical_mapping_init(, init = true)
		phys_p4d_init(, init = true)
			phys_pud_init(, init = true)
				phys_pmd_init(, init = true)
					phys_pte_init(, init = true)
						pte_init(, init = true)
							set_pte_safe()

Right, seems like there is a path to set_pte_safe(). Sure, will drop this
patch and convert the direct access into ptep_get() instead.
kernel test robot Sept. 13, 2024, 4:16 a.m. UTC | #3 
Hi Anshuman,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Anshuman-Khandual/mm-Drop-unused-set_pte_safe/20240910-181151
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240910101026.428808-1-anshuman.khandual%40arm.com
patch subject: [PATCH V2] mm: Drop unused set_pte_safe()
config: x86_64-allnoconfig (https://download.01.org/0day-ci/archive/20240913/202409131220.CJ5MlGCG-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240913/202409131220.CJ5MlGCG-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/202409131220.CJ5MlGCG-lkp@intel.com/

All errors (new ones prefixed by >>):

>> arch/x86/mm/init_64.c:91:1: error: call to undeclared function 'set_pte_safe'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
      91 | DEFINE_ENTRY(pte, pte, init)
         | ^
   arch/x86/mm/init_64.c:83:3: note: expanded from macro 'DEFINE_ENTRY'
      83 |                 set_##type1##_safe(arg1, arg2);                 \
         |                 ^
   <scratch space>:49:1: note: expanded from here
      49 | set_pte_safe
         | ^
   arch/x86/mm/init_64.c:91:1: note: did you mean 'set_pte_range'?
   arch/x86/mm/init_64.c:83:3: note: expanded from macro 'DEFINE_ENTRY'
      83 |                 set_##type1##_safe(arg1, arg2);                 \
         |                 ^
   <scratch space>:49:1: note: expanded from here
      49 | set_pte_safe
         | ^
   include/linux/mm.h:1331:6: note: 'set_pte_range' declared here
    1331 | void set_pte_range(struct vm_fault *vmf, struct folio *folio,
         |      ^
   1 error generated.


vim +/set_pte_safe +91 arch/x86/mm/init_64.c

eccd906484d1cd Brijesh Singh 2019-04-17  87  
eccd906484d1cd Brijesh Singh 2019-04-17  88  DEFINE_ENTRY(p4d, p4d, init)
eccd906484d1cd Brijesh Singh 2019-04-17  89  DEFINE_ENTRY(pud, pud, init)
eccd906484d1cd Brijesh Singh 2019-04-17  90  DEFINE_ENTRY(pmd, pmd, init)
eccd906484d1cd Brijesh Singh 2019-04-17 @91  DEFINE_ENTRY(pte, pte, init)
eccd906484d1cd Brijesh Singh 2019-04-17  92
kernel test robot Sept. 13, 2024, 4:37 a.m. UTC | #4 
Hi Anshuman,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Anshuman-Khandual/mm-Drop-unused-set_pte_safe/20240910-181151
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240910101026.428808-1-anshuman.khandual%40arm.com
patch subject: [PATCH V2] mm: Drop unused set_pte_safe()
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20240913/202409131214.d09v9mIO-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240913/202409131214.d09v9mIO-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/202409131214.d09v9mIO-lkp@intel.com/

All errors (new ones prefixed by >>):

   arch/x86/mm/init_64.c: In function 'set_pte_init':
>> arch/x86/mm/init_64.c:83:17: error: implicit declaration of function 'set_pte_safe'; did you mean 'set_pud_safe'? [-Werror=implicit-function-declaration]
      83 |                 set_##type1##_safe(arg1, arg2);                 \
         |                 ^~~~
   arch/x86/mm/init_64.c:91:1: note: in expansion of macro 'DEFINE_ENTRY'
      91 | DEFINE_ENTRY(pte, pte, init)
         | ^~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +83 arch/x86/mm/init_64.c

eccd906484d1cd Brijesh Singh 2019-04-17  77  
eccd906484d1cd Brijesh Singh 2019-04-17  78  #define DEFINE_ENTRY(type1, type2, init)			\
eccd906484d1cd Brijesh Singh 2019-04-17  79  static inline void set_##type1##_init(type1##_t *arg1,		\
eccd906484d1cd Brijesh Singh 2019-04-17  80  			type2##_t arg2, bool init)		\
eccd906484d1cd Brijesh Singh 2019-04-17  81  {								\
eccd906484d1cd Brijesh Singh 2019-04-17  82  	if (init)						\
eccd906484d1cd Brijesh Singh 2019-04-17 @83  		set_##type1##_safe(arg1, arg2);			\
eccd906484d1cd Brijesh Singh 2019-04-17  84  	else							\
eccd906484d1cd Brijesh Singh 2019-04-17  85  		set_##type1(arg1, arg2);			\
eccd906484d1cd Brijesh Singh 2019-04-17  86  }
eccd906484d1cd Brijesh Singh 2019-04-17  87
diff mbox series

Patch

diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index 2a6a3cccfc36..aeabbf0db7c8 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -1058,12 +1058,6 @@  static inline int pgd_same(pgd_t pgd_a, pgd_t pgd_b)
  * same value. Otherwise, use the typical "set" helpers and flush the
  * TLB.
  */
-#define set_pte_safe(ptep, pte) \
-({ \
-	WARN_ON_ONCE(pte_present(*ptep) && !pte_same(*ptep, pte)); \
-	set_pte(ptep, pte); \
-})
-
 #define set_pmd_safe(pmdp, pmd) \
 ({ \
 	WARN_ON_ONCE(pmd_present(*pmdp) && !pmd_same(*pmdp, pmd)); \