diff mbox

parisc: Optimize switch_mm

Message ID 20170726180252.GA1129@p100.box (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Helge Deller July 26, 2017, 6:02 p.m. UTC
* John David Anglin <dave.anglin@bell.net>:
> We only need to switch contexts when prev != next, and we don't need to disable interrupts
> to do the check.
> 
> Signed-off-by: John David Anglin <dave.anglin@bell.net>
> 
> 

> diff --git a/arch/parisc/include/asm/mmu_context.h b/arch/parisc/include/asm/mmu_context.h
> index a81226257878..b1dc6e3f0dd0 100644
> --- a/arch/parisc/include/asm/mmu_context.h
> +++ b/arch/parisc/include/asm/mmu_context.h
> @@ -49,26 +49,19 @@ static inline void load_context(mm_context_t context)
>  	mtctl(__space_to_prot(context), 8);
>  }
>  
> -static inline void switch_mm_irqs_off(struct mm_struct *prev,
> +static inline void switch_mm(struct mm_struct *prev,
>  		struct mm_struct *next, struct task_struct *tsk)
>  {
> +	unsigned long flags;
> +
>  	if (prev != next) {
> +		local_irq_save(flags);
>  		mtctl(__pa(next->pgd), 25);
>  		load_context(next->context);
> +		local_irq_restore(flags);
>  	}
>  }
>  
> -static inline void switch_mm(struct mm_struct *prev,
> -		struct mm_struct *next, struct task_struct *tsk)
> -{
> -	unsigned long flags;
> -
> -	local_irq_save(flags);
> -	switch_mm_irqs_off(prev, next, tsk);
> -	local_irq_restore(flags);
> -}
> -#define switch_mm_irqs_off switch_mm_irqs_off
> -
>  #define deactivate_mm(tsk,mm)	do { } while (0)
>  
>  static inline void activate_mm(struct mm_struct *prev, struct mm_struct *next)


Instead I'd then suggest the patch below.
The if-clause in switch_mm_irqs_off() will then probably be optimized
away by the compiler.

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

John David Anglin July 26, 2017, 6:49 p.m. UTC | #1
On 2017-07-26, at 2:02 PM, Helge Deller wrote:

> Instead I'd then suggest the patch below.
> The if-clause in switch_mm_irqs_off() will then probably be optimized
> away by the compiler.
> 
> diff --git a/arch/parisc/include/asm/mmu_context.h b/arch/parisc/include/asm/mmu_context.h
> index a812262..e4a6570 100644
> --- a/arch/parisc/include/asm/mmu_context.h
> +++ b/arch/parisc/include/asm/mmu_context.h
> @@ -63,6 +63,9 @@ static inline void switch_mm(struct mm_struct *prev,
> {
> 	unsigned long flags;
> 
> +	if (prev == next)
> +		return;
> +
> 	local_irq_save(flags);
> 	switch_mm_irqs_off(prev, next, tsk);
> 	local_irq_restore(flags);

You are correct.  I missed the fact that switch_mm_irqs_off() is used by kernel/sched/core.c.

Maybe we should replace the "switch_mm_irqs_off(prev, next, tsk);" with the mtctl and load_context
lines to ensure that the if-clause is optimized?

Dave
--
John David Anglin	dave.anglin@bell.net



--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Helge Deller July 26, 2017, 7:13 p.m. UTC | #2
On 26.07.2017 20:49, John David Anglin wrote:
> On 2017-07-26, at 2:02 PM, Helge Deller wrote:
> 
>> Instead I'd then suggest the patch below.
>> The if-clause in switch_mm_irqs_off() will then probably be optimized
>> away by the compiler.
>>
>> diff --git a/arch/parisc/include/asm/mmu_context.h b/arch/parisc/include/asm/mmu_context.h
>> index a812262..e4a6570 100644
>> --- a/arch/parisc/include/asm/mmu_context.h
>> +++ b/arch/parisc/include/asm/mmu_context.h
>> @@ -63,6 +63,9 @@ static inline void switch_mm(struct mm_struct *prev,
>> {
>> 	unsigned long flags;
>>
>> +	if (prev == next)
>> +		return;
>> +
>> 	local_irq_save(flags);
>> 	switch_mm_irqs_off(prev, next, tsk);
>> 	local_irq_restore(flags);
> 
> You are correct.  I missed the fact that switch_mm_irqs_off() is used by kernel/sched/core.c.
> 
> Maybe we should replace the "switch_mm_irqs_off(prev, next, tsk);" with the mtctl and load_context
> lines to ensure that the if-clause is optimized?

I think we should look at the generated assembly, and if it's not optimized away replace it with mtctl/load_context.

Helge
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/parisc/include/asm/mmu_context.h b/arch/parisc/include/asm/mmu_context.h
index a812262..e4a6570 100644
--- a/arch/parisc/include/asm/mmu_context.h
+++ b/arch/parisc/include/asm/mmu_context.h
@@ -63,6 +63,9 @@  static inline void switch_mm(struct mm_struct *prev,
 {
 	unsigned long flags;
 
+	if (prev == next)
+		return;
+
 	local_irq_save(flags);
 	switch_mm_irqs_off(prev, next, tsk);
 	local_irq_restore(flags);