diff mbox series

[-next,v5,2/5] mm: page_table_check: move pxx_user_accessible_page into x86

Message ID 20220421082042.1167967-3-tongtiangen@huawei.com (mailing list archive)
State New
Headers show
Series mm: page_table_check: add support on arm64 and riscv | expand

Commit Message

Tong Tiangen April 21, 2022, 8:20 a.m. UTC 
From: Kefeng Wang <wangkefeng.wang@huawei.com>

The pxx_user_accessible_page() check the PTE bit, it's
architecture-specific code, move them into x86's pgtable.h,

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
---
 arch/x86/include/asm/pgtable.h | 19 +++++++++++++++++++
 mm/page_table_check.c          | 17 -----------------
 2 files changed, 19 insertions(+), 17 deletions(-)

Comments

Anshuman Khandual April 22, 2022, 5:11 a.m. UTC | #1 
Similar to previous commits on the same file, the following subject
line format, would have been preferred.

mm/page_table_check: <description>

On 4/21/22 13:50, Tong Tiangen wrote:
> From: Kefeng Wang <wangkefeng.wang@huawei.com>
> 
> The pxx_user_accessible_page() check the PTE bit, it's

s/check/checks			 ^^^^

> architecture-specific code, move them into x86's pgtable.h
The commit message should have been more clear, atleast complete in
sentences. I dont want to be bike shedding here but this is definitely
incomplete. These helpers are being moved out to make the page table
check framework, platform independent. Hence the commit message should
mention this.

> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
>  arch/x86/include/asm/pgtable.h | 19 +++++++++++++++++++
>  mm/page_table_check.c          | 17 -----------------
>  2 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index b7464f13e416..564abe42b0f7 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -1447,6 +1447,25 @@ static inline bool arch_has_hw_pte_young(void)
>  	return true;
>  }
>  
> +#ifdef CONFIG_PAGE_TABLE_CHECK
> +static inline bool pte_user_accessible_page(pte_t pte)
> +{
> +	return (pte_val(pte) & _PAGE_PRESENT) && (pte_val(pte) & _PAGE_USER);
> +}
> +
> +static inline bool pmd_user_accessible_page(pmd_t pmd)
> +{
> +	return pmd_leaf(pmd) && (pmd_val(pmd) & _PAGE_PRESENT) &&
> +		(pmd_val(pmd) & _PAGE_USER);
> +}
> +
> +static inline bool pud_user_accessible_page(pud_t pud)
> +{
> +	return pud_leaf(pud) && (pud_val(pud) & _PAGE_PRESENT) &&
> +		(pud_val(pud) & _PAGE_USER);

A line break is not really required here (and above as well). As single
complete line would still be within 100 characters.

> +}
> +#endif
> +
>  #endif	/* __ASSEMBLY__ */
>  
>  #endif /* _ASM_X86_PGTABLE_H */
> diff --git a/mm/page_table_check.c b/mm/page_table_check.c
> index eb0d0b71cdf6..3692bea2ea2c 100644
> --- a/mm/page_table_check.c
> +++ b/mm/page_table_check.c
> @@ -52,23 +52,6 @@ static struct page_table_check *get_page_table_check(struct page_ext *page_ext)
>  	return (void *)(page_ext) + page_table_check_ops.offset;
>  }
>  
> -static inline bool pte_user_accessible_page(pte_t pte)
> -{
> -	return (pte_val(pte) & _PAGE_PRESENT) && (pte_val(pte) & _PAGE_USER);
> -}
> -
> -static inline bool pmd_user_accessible_page(pmd_t pmd)
> -{
> -	return pmd_leaf(pmd) && (pmd_val(pmd) & _PAGE_PRESENT) &&
> -		(pmd_val(pmd) & _PAGE_USER);
> -}
> -
> -static inline bool pud_user_accessible_page(pud_t pud)
> -{
> -	return pud_leaf(pud) && (pud_val(pud) & _PAGE_PRESENT) &&
> -		(pud_val(pud) & _PAGE_USER);
> -}
> -
>  /*
>   * An enty is removed from the page table, decrement the counters for that page
>   * verify that it is of correct type and counters do not become negative.

With above mentioned code cleanup and commit message changes in place.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Tong Tiangen April 22, 2022, 6:30 a.m. UTC | #2 
在 2022/4/22 13:11, Anshuman Khandual 写道:
> Similar to previous commits on the same file, the following subject
> line format, would have been preferred.
> 
> mm/page_table_check: <description>
> 
> On 4/21/22 13:50, Tong Tiangen wrote:
>> From: Kefeng Wang <wangkefeng.wang@huawei.com>
>>
>> The pxx_user_accessible_page() check the PTE bit, it's
> 
> s/check/checks			 ^^^^
> 
>> architecture-specific code, move them into x86's pgtable.h
> The commit message should have been more clear, atleast complete in
> sentences. I dont want to be bike shedding here but this is definitely
> incomplete. These helpers are being moved out to make the page table
> check framework, platform independent. Hence the commit message should
> mention this.

The commit message is not very clear and it is too simple.

Thanks.

> 
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
>> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
>> ---
>>   arch/x86/include/asm/pgtable.h | 19 +++++++++++++++++++
>>   mm/page_table_check.c          | 17 -----------------
>>   2 files changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
>> index b7464f13e416..564abe42b0f7 100644
>> --- a/arch/x86/include/asm/pgtable.h
>> +++ b/arch/x86/include/asm/pgtable.h
>> @@ -1447,6 +1447,25 @@ static inline bool arch_has_hw_pte_young(void)
>>   	return true;
>>   }
>>   
>> +#ifdef CONFIG_PAGE_TABLE_CHECK
>> +static inline bool pte_user_accessible_page(pte_t pte)
>> +{
>> +	return (pte_val(pte) & _PAGE_PRESENT) && (pte_val(pte) & _PAGE_USER);
>> +}
>> +
>> +static inline bool pmd_user_accessible_page(pmd_t pmd)
>> +{
>> +	return pmd_leaf(pmd) && (pmd_val(pmd) & _PAGE_PRESENT) &&
>> +		(pmd_val(pmd) & _PAGE_USER);
>> +}
>> +
>> +static inline bool pud_user_accessible_page(pud_t pud)
>> +{
>> +	return pud_leaf(pud) && (pud_val(pud) & _PAGE_PRESENT) &&
>> +		(pud_val(pud) & _PAGE_USER);
> 
> A line break is not really required here (and above as well). As single
> complete line would still be within 100 characters.
> 
Right, now one line can have 100 characters.

Thanks.

>> +}
>> +#endif
>> +
>>   #endif	/* __ASSEMBLY__ */
>>   
>>   #endif /* _ASM_X86_PGTABLE_H */
>> diff --git a/mm/page_table_check.c b/mm/page_table_check.c
>> index eb0d0b71cdf6..3692bea2ea2c 100644
>> --- a/mm/page_table_check.c
>> +++ b/mm/page_table_check.c
>> @@ -52,23 +52,6 @@ static struct page_table_check *get_page_table_check(struct page_ext *page_ext)
>>   	return (void *)(page_ext) + page_table_check_ops.offset;
>>   }
>>   
>> -static inline bool pte_user_accessible_page(pte_t pte)
>> -{
>> -	return (pte_val(pte) & _PAGE_PRESENT) && (pte_val(pte) & _PAGE_USER);
>> -}
>> -
>> -static inline bool pmd_user_accessible_page(pmd_t pmd)
>> -{
>> -	return pmd_leaf(pmd) && (pmd_val(pmd) & _PAGE_PRESENT) &&
>> -		(pmd_val(pmd) & _PAGE_USER);
>> -}
>> -
>> -static inline bool pud_user_accessible_page(pud_t pud)
>> -{
>> -	return pud_leaf(pud) && (pud_val(pud) & _PAGE_PRESENT) &&
>> -		(pud_val(pud) & _PAGE_USER);
>> -}
>> -
>>   /*
>>    * An enty is removed from the page table, decrement the counters for that page
>>    * verify that it is of correct type and counters do not become negative.
> 
> With above mentioned code cleanup and commit message changes in place.
> 
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> .
diff mbox series

Patch

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index b7464f13e416..564abe42b0f7 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -1447,6 +1447,25 @@  static inline bool arch_has_hw_pte_young(void)
 	return true;
 }
 
+#ifdef CONFIG_PAGE_TABLE_CHECK
+static inline bool pte_user_accessible_page(pte_t pte)
+{
+	return (pte_val(pte) & _PAGE_PRESENT) && (pte_val(pte) & _PAGE_USER);
+}
+
+static inline bool pmd_user_accessible_page(pmd_t pmd)
+{
+	return pmd_leaf(pmd) && (pmd_val(pmd) & _PAGE_PRESENT) &&
+		(pmd_val(pmd) & _PAGE_USER);
+}
+
+static inline bool pud_user_accessible_page(pud_t pud)
+{
+	return pud_leaf(pud) && (pud_val(pud) & _PAGE_PRESENT) &&
+		(pud_val(pud) & _PAGE_USER);
+}
+#endif
+
 #endif	/* __ASSEMBLY__ */
 
 #endif /* _ASM_X86_PGTABLE_H */
diff --git a/mm/page_table_check.c b/mm/page_table_check.c
index eb0d0b71cdf6..3692bea2ea2c 100644
--- a/mm/page_table_check.c
+++ b/mm/page_table_check.c
@@ -52,23 +52,6 @@  static struct page_table_check *get_page_table_check(struct page_ext *page_ext)
 	return (void *)(page_ext) + page_table_check_ops.offset;
 }
 
-static inline bool pte_user_accessible_page(pte_t pte)
-{
-	return (pte_val(pte) & _PAGE_PRESENT) && (pte_val(pte) & _PAGE_USER);
-}
-
-static inline bool pmd_user_accessible_page(pmd_t pmd)
-{
-	return pmd_leaf(pmd) && (pmd_val(pmd) & _PAGE_PRESENT) &&
-		(pmd_val(pmd) & _PAGE_USER);
-}
-
-static inline bool pud_user_accessible_page(pud_t pud)
-{
-	return pud_leaf(pud) && (pud_val(pud) & _PAGE_PRESENT) &&
-		(pud_val(pud) & _PAGE_USER);
-}
-
 /*
  * An enty is removed from the page table, decrement the counters for that page
  * verify that it is of correct type and counters do not become negative.