All of lore.kernel.org
 help / color / mirror / Atom feed
From: catalin.marinas@arm.com (Catalin Marinas)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: Fix preemption disable in iwmmxt_task_enable()
Date: Fri, 11 Jul 2014 18:09:36 +0100	[thread overview]
Message-ID: <20140711170935.GG16321@arm.com> (raw)
In-Reply-To: <1405069813-9270-1-git-send-email-sebastian.hesselbarth@gmail.com>

On Fri, Jul 11, 2014 at 10:10:13AM +0100, Sebastian Hesselbarth wrote:
> commit 431a84b1a4f7d1a0085d5b91330c5053cc8e8b12
>  ("ARM: 8034/1: Disable preemption in iwmmxt_task_enable()")
> introduced macros {inc,dec}_preempt_count to iwmmxt_task_enable
> to make it run with preemption disabled.
> 
> Unfortunately, other functions in iwmmxt.S also use concan_{save,dump,load}
> sections located in iwmmxt_task_enable() to deal with iWMMXt coprocessor.
> This causes an unbalanced preempt_count due to excessive dec_preempt_count
> and destroyed return addresses in callers of concan_ labels due to a register
> collision:

Indeed, I missed this part completely.

> --- a/arch/arm/kernel/iwmmxt.S
> +++ b/arch/arm/kernel/iwmmxt.S
> @@ -94,13 +94,19 @@ ENTRY(iwmmxt_task_enable)
>  
>  	mrc	p15, 0, r2, c2, c0, 0
>  	mov	r2, r2				@ cpwait
> +	bl	concan_save
>  
> -	teq	r1, #0				@ test for last ownership
> -	mov	lr, r9				@ normal exit from exception
> -	beq	concan_load			@ no owner, skip save
> +#ifdef CONFIG_PREEMPT_COUNT
> +	get_thread_info r10
> +#endif
> +4:	dec_preempt_count r10, r3
> +	mov	pc, r9				@ normal exit from exception
>  
>  concan_save:
>  
> +	teq	r1, #0				@ test for last ownership
> +	beq	concan_load			@ no owner, skip save
> +
>  	tmrc	r2, wCon
>  
>  	@ CUP? wCx
> @@ -175,10 +181,6 @@ concan_load:
>  	tmcr	wCon, r2
>  
>  3:
> -#ifdef CONFIG_PREEMPT_COUNT
> -	get_thread_info r10
> -#endif
> -4:	dec_preempt_count r10, r3
>  	mov	pc, lr

It looks fine to me. One optimisation you could do is to replace a
couple of beq 3f with moveq pc, lr.

-- 
Catalin

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>,
	Jean-Francois Moine <moinejf@free.fr>,
	Jason Cooper <jason@lakedaemon.net>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ARM: Fix preemption disable in iwmmxt_task_enable()
Date: Fri, 11 Jul 2014 18:09:36 +0100	[thread overview]
Message-ID: <20140711170935.GG16321@arm.com> (raw)
In-Reply-To: <1405069813-9270-1-git-send-email-sebastian.hesselbarth@gmail.com>

On Fri, Jul 11, 2014 at 10:10:13AM +0100, Sebastian Hesselbarth wrote:
> commit 431a84b1a4f7d1a0085d5b91330c5053cc8e8b12
>  ("ARM: 8034/1: Disable preemption in iwmmxt_task_enable()")
> introduced macros {inc,dec}_preempt_count to iwmmxt_task_enable
> to make it run with preemption disabled.
> 
> Unfortunately, other functions in iwmmxt.S also use concan_{save,dump,load}
> sections located in iwmmxt_task_enable() to deal with iWMMXt coprocessor.
> This causes an unbalanced preempt_count due to excessive dec_preempt_count
> and destroyed return addresses in callers of concan_ labels due to a register
> collision:

Indeed, I missed this part completely.

> --- a/arch/arm/kernel/iwmmxt.S
> +++ b/arch/arm/kernel/iwmmxt.S
> @@ -94,13 +94,19 @@ ENTRY(iwmmxt_task_enable)
>  
>  	mrc	p15, 0, r2, c2, c0, 0
>  	mov	r2, r2				@ cpwait
> +	bl	concan_save
>  
> -	teq	r1, #0				@ test for last ownership
> -	mov	lr, r9				@ normal exit from exception
> -	beq	concan_load			@ no owner, skip save
> +#ifdef CONFIG_PREEMPT_COUNT
> +	get_thread_info r10
> +#endif
> +4:	dec_preempt_count r10, r3
> +	mov	pc, r9				@ normal exit from exception
>  
>  concan_save:
>  
> +	teq	r1, #0				@ test for last ownership
> +	beq	concan_load			@ no owner, skip save
> +
>  	tmrc	r2, wCon
>  
>  	@ CUP? wCx
> @@ -175,10 +181,6 @@ concan_load:
>  	tmcr	wCon, r2
>  
>  3:
> -#ifdef CONFIG_PREEMPT_COUNT
> -	get_thread_info r10
> -#endif
> -4:	dec_preempt_count r10, r3
>  	mov	pc, lr

It looks fine to me. One optimisation you could do is to replace a
couple of beq 3f with moveq pc, lr.

-- 
Catalin

  reply	other threads:[~2014-07-11 17:09 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-06  6:08 dove (marvell A510) crash on boot with config_preempt Jean-Francois Moine
2014-07-08 15:17 ` Jason Cooper
2014-07-10 12:33   ` Sebastian Hesselbarth
2014-07-10 20:55     ` Sebastian Hesselbarth
2014-07-10 22:13       ` Russell King - ARM Linux
2014-07-10 22:08 ` Russell King - ARM Linux
2014-07-11  9:10 ` [PATCH] ARM: Fix preemption disable in iwmmxt_task_enable() Sebastian Hesselbarth
2014-07-11  9:10   ` Sebastian Hesselbarth
2014-07-11 17:09   ` Catalin Marinas [this message]
2014-07-11 17:09     ` Catalin Marinas
2014-07-12 11:05   ` [PATCH v2] " Sebastian Hesselbarth
2014-07-12 11:05     ` Sebastian Hesselbarth
2014-07-14 12:08     ` Catalin Marinas
2014-07-14 12:08       ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140711170935.GG16321@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.