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
next prev parent 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.