diff mbox series

[06/18] ARC: mm: Enable STRICT_MM_TYPECHECKS

Message ID 20210811004258.138075-7-vgupta@kernel.org
State New
Headers show
Series ARC mm updates to support 3 or 4 levels of paging | expand

Commit Message

Vineet Gupta Aug. 11, 2021, 12:42 a.m. UTC
In the past I've refrained from doing this (atleast 2 times) due to the
slight code bloat due to ABI implications of pte_t etc becoming sttuct

Per ARC ABI, functions return struct via memory and not through register
r0, even if the struct would fits in register(s)

 - caller allocates space on stack and passes the address as first arg
   (r0), shifting rest of args by one

 - callee creates return struct in memory (referenced via r0)

This time around the code actually shrunk slightly (due to subtle
inlining heuristic effects), but still slightly inefficient due to
return values passed through memory. That however seems like a small
cost compared to maintenance burden given the impending new mmu support
for page walk etc

Signed-off-by: Vineet Gupta <vgupta@kernel.org>
---
 arch/arc/include/asm/page.h | 26 --------------------------
 arch/arc/mm/ioremap.c       |  2 +-
 2 files changed, 1 insertion(+), 27 deletions(-)

Comments

Mike Rapoport Aug. 11, 2021, 12:04 p.m. UTC | #1
On Tue, Aug 10, 2021 at 05:42:46PM -0700, Vineet Gupta wrote:
> In the past I've refrained from doing this (atleast 2 times) due to the

                                            ^ at least

> slight code bloat due to ABI implications of pte_t etc becoming sttuct

                                                                 ^ struct
> 
> Per ARC ABI, functions return struct via memory and not through register
> r0, even if the struct would fits in register(s)

                              ^ fit
> 
>  - caller allocates space on stack and passes the address as first arg
>    (r0), shifting rest of args by one
> 
>  - callee creates return struct in memory (referenced via r0)
> 
> This time around the code actually shrunk slightly (due to subtle
> inlining heuristic effects), but still slightly inefficient due to
> return values passed through memory. That however seems like a small

Out of curiosity, is this actually measurable on real world applications?

> cost compared to maintenance burden given the impending new mmu support
> for page walk etc
> 
> Signed-off-by: Vineet Gupta <vgupta@kernel.org>
> ---
>  arch/arc/include/asm/page.h | 26 --------------------------
>  arch/arc/mm/ioremap.c       |  2 +-
>  2 files changed, 1 insertion(+), 27 deletions(-)
> 
> diff --git a/arch/arc/include/asm/page.h b/arch/arc/include/asm/page.h
> index 4a9d33372fe2..c4ac827379cd 100644
> --- a/arch/arc/include/asm/page.h
> +++ b/arch/arc/include/asm/page.h
> @@ -34,12 +34,6 @@ void copy_user_highpage(struct page *to, struct page *from,
>  			unsigned long u_vaddr, struct vm_area_struct *vma);
>  void clear_user_page(void *to, unsigned long u_vaddr, struct page *page);
>  
> -#undef STRICT_MM_TYPECHECKS
> -
> -#ifdef STRICT_MM_TYPECHECKS
> -/*
> - * These are used to make use of C type-checking..
> - */
>  typedef struct {
>  #ifdef CONFIG_ARC_HAS_PAE40
>  	unsigned long long pte;
> @@ -64,26 +58,6 @@ typedef struct {
>  
>  #define pte_pgprot(x) __pgprot(pte_val(x))
>  
> -#else /* !STRICT_MM_TYPECHECKS */
> -
> -#ifdef CONFIG_ARC_HAS_PAE40
> -typedef unsigned long long pte_t;
> -#else
> -typedef unsigned long pte_t;
> -#endif
> -typedef unsigned long pgd_t;
> -typedef unsigned long pgprot_t;
> -
> -#define pte_val(x)	(x)
> -#define pgd_val(x)	(x)
> -#define pgprot_val(x)	(x)
> -#define __pte(x)	(x)
> -#define __pgd(x)	(x)
> -#define __pgprot(x)	(x)
> -#define pte_pgprot(x)	(x)
> -
> -#endif
> -
>  typedef pte_t * pgtable_t;
>  
>  /*
> diff --git a/arch/arc/mm/ioremap.c b/arch/arc/mm/ioremap.c
> index 95c649fbc95a..052bbd8b1e5f 100644
> --- a/arch/arc/mm/ioremap.c
> +++ b/arch/arc/mm/ioremap.c
> @@ -39,7 +39,7 @@ void __iomem *ioremap(phys_addr_t paddr, unsigned long size)
>  	if (arc_uncached_addr_space(paddr))
>  		return (void __iomem *)(u32)paddr;
>  
> -	return ioremap_prot(paddr, size, PAGE_KERNEL_NO_CACHE);
> +	return ioremap_prot(paddr, size, pgprot_val(PAGE_KERNEL_NO_CACHE));
>  }
>  EXPORT_SYMBOL(ioremap);
>  
> -- 
> 2.25.1
> 
>
Vineet Gupta Aug. 11, 2021, 7:01 p.m. UTC | #2
On 8/11/21 5:04 AM, Mike Rapoport wrote:
> On Tue, Aug 10, 2021 at 05:42:46PM -0700, Vineet Gupta wrote:
>> In the past I've refrained from doing this (atleast 2 times) due to the
>                                              ^ at least
>
>> slight code bloat due to ABI implications of pte_t etc becoming sttuct
>                                                                   ^ struct
>> Per ARC ABI, functions return struct via memory and not through register
>> r0, even if the struct would fits in register(s)
>                                ^ fit

Fixed.

>>   - caller allocates space on stack and passes the address as first arg
>>     (r0), shifting rest of args by one
>>
>>   - callee creates return struct in memory (referenced via r0)
>>
>> This time around the code actually shrunk slightly (due to subtle
>> inlining heuristic effects), but still slightly inefficient due to
>> return values passed through memory. That however seems like a small
> Out of curiosity, is this actually measurable on real world applications?

Not really, but they look hideous to an objdump hunkie like me (who 
treats waste of electrons like wasting food) and wondering what piece of 
art your compiler is spitting out ;-) only to realize that is source 
code induced.
diff mbox series

Patch

diff --git a/arch/arc/include/asm/page.h b/arch/arc/include/asm/page.h
index 4a9d33372fe2..c4ac827379cd 100644
--- a/arch/arc/include/asm/page.h
+++ b/arch/arc/include/asm/page.h
@@ -34,12 +34,6 @@  void copy_user_highpage(struct page *to, struct page *from,
 			unsigned long u_vaddr, struct vm_area_struct *vma);
 void clear_user_page(void *to, unsigned long u_vaddr, struct page *page);
 
-#undef STRICT_MM_TYPECHECKS
-
-#ifdef STRICT_MM_TYPECHECKS
-/*
- * These are used to make use of C type-checking..
- */
 typedef struct {
 #ifdef CONFIG_ARC_HAS_PAE40
 	unsigned long long pte;
@@ -64,26 +58,6 @@  typedef struct {
 
 #define pte_pgprot(x) __pgprot(pte_val(x))
 
-#else /* !STRICT_MM_TYPECHECKS */
-
-#ifdef CONFIG_ARC_HAS_PAE40
-typedef unsigned long long pte_t;
-#else
-typedef unsigned long pte_t;
-#endif
-typedef unsigned long pgd_t;
-typedef unsigned long pgprot_t;
-
-#define pte_val(x)	(x)
-#define pgd_val(x)	(x)
-#define pgprot_val(x)	(x)
-#define __pte(x)	(x)
-#define __pgd(x)	(x)
-#define __pgprot(x)	(x)
-#define pte_pgprot(x)	(x)
-
-#endif
-
 typedef pte_t * pgtable_t;
 
 /*
diff --git a/arch/arc/mm/ioremap.c b/arch/arc/mm/ioremap.c
index 95c649fbc95a..052bbd8b1e5f 100644
--- a/arch/arc/mm/ioremap.c
+++ b/arch/arc/mm/ioremap.c
@@ -39,7 +39,7 @@  void __iomem *ioremap(phys_addr_t paddr, unsigned long size)
 	if (arc_uncached_addr_space(paddr))
 		return (void __iomem *)(u32)paddr;
 
-	return ioremap_prot(paddr, size, PAGE_KERNEL_NO_CACHE);
+	return ioremap_prot(paddr, size, pgprot_val(PAGE_KERNEL_NO_CACHE));
 }
 EXPORT_SYMBOL(ioremap);