[ORLinux] [PATCH] openrisc: call do_notify_resume() with interrupts enabled

[Stefan Kristiansson]

A signal delivered through do_notify_resume() would cause the
irqs_disabled() check in _local_bh_enable_ip() to be triggered.

Enable interrupts before calling do_notify_resume().

Signed-off-by: Stefan Kristiansson <stefan.kristiansson at saunalahti.fi>
---
arch/openrisc/kernel/entry.S | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index d8a455e..55ffc97 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
@@ -874,6 +874,7 @@ _work_pending:
* must be set so that the syscall restart functionality works.
*/
_work_notifysig:
+ ENABLE_INTERRUPTS(r29)
l.jal do_notify_resume
l.ori r3,r1,0 /* pt_regs */

[Stefan Kristiansson]

Post by Stefan Kristiansson
A signal delivered through do_notify_resume() would cause the
irqs_disabled() check in _local_bh_enable_ip() to be triggered.
Enable interrupts before calling do_notify_resume().
Signed-off-by: Stefan Kristiansson <stefan.kristiansson at saunalahti.fi>
---
arch/openrisc/kernel/entry.S | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index d8a455e..55ffc97 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
* must be set so that the syscall restart functionality works.
*/
+ ENABLE_INTERRUPTS(r29)
l.jal do_notify_resume
l.ori r3,r1,0 /* pt_regs */

I just realised that this one-liner isn't quite enough,
the flags that are used in do_notify_resume() has to be read with
interrupts off.
Revised patch coming up.

Stefan

[Stefan Kristiansson]

A signal delivered through do_notify_resume() would cause the
irqs_disabled() check in _local_bh_enable_ip() to be triggered.

Enable interrupts before calling do_notify_resume(), but read
the current_thread_info->flags before doing so.

Signed-off-by: Stefan Kristiansson <stefan.kristiansson at saunalahti.fi>
---
arch/openrisc/kernel/entry.S | 5 +++--
arch/openrisc/kernel/signal.c | 7 ++++---
2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index d8a455e..7564784 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
@@ -858,8 +858,8 @@ _work_pending:
* if (current_thread_info->flags & _TIF_NEED_RESCHED)
* schedule();
*/
- l.lwz r5,TI_FLAGS(r10)
- l.andi r3,r5,_TIF_NEED_RESCHED
+ l.lwz r4,TI_FLAGS(r10)
+ l.andi r3,r4,_TIF_NEED_RESCHED
l.sfnei r3,0
l.bnf _work_notifysig
l.nop
@@ -874,6 +874,7 @@ _work_pending:
* must be set so that the syscall restart functionality works.
*/
_work_notifysig:
+ ENABLE_INTERRUPTS(r29)
l.jal do_notify_resume
l.ori r3,r1,0 /* pt_regs */

diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
index ae167f7..032bed1 100644
--- a/arch/openrisc/kernel/signal.c
+++ b/arch/openrisc/kernel/signal.c
@@ -331,12 +331,13 @@ void do_signal(struct pt_regs *regs)
return;
}

-asmlinkage void do_notify_resume(struct pt_regs *regs)
+asmlinkage void do_notify_resume(struct pt_regs *regs,
+ unsigned long thread_flags)
{
- if (current_thread_info()->flags & _TIF_SIGPENDING)
+ if (thread_flags & _TIF_SIGPENDING)
do_signal(regs);

- if (current_thread_info()->flags & _TIF_NOTIFY_RESUME) {
+ if (thread_flags & _TIF_NOTIFY_RESUME) {
clear_thread_flag(TIF_NOTIFY_RESUME);
tracehook_notify_resume(regs);
}

[Sebastian Macke]

Post by Stefan Kristiansson
A signal delivered through do_notify_resume() would cause the
irqs_disabled() check in _local_bh_enable_ip() to be triggered.
Enable interrupts before calling do_notify_resume(), but read
the current_thread_info->flags before doing so.
Signed-off-by: Stefan Kristiansson <stefan.kristiansson at saunalahti.fi>

Tested-by: Sebastian Macke <sebastian at macke.de>

The warning during the booting process is gone and
the kernel does no longer crash while executing the dup06 test case of
the ltp project.
http://ltp.cvs.sourceforge.net/viewvc/ltp/ltp/testcases/kernel/syscalls/dup/dup06.c?view=markup

I would suggest to apply the patch.

[Jonas Bonn]

Hi Stefan,

Thanks for this patch. I intend to push another patch that reworks our
signal handling code instead as it happens to have the same fix (though
in another form).

Please grab 'master' from my tree and check that it works for you. See
commit aeca89e052a618ddf9f02d55dac6684e8e0eef78.

/Jonas

On 04/29/2013 12:40 PM, Stefan Kristiansson wrote:> A signal delivered
through do_notify_resume() would cause the

Post by Stefan Kristiansson
irqs_disabled() check in _local_bh_enable_ip() to be triggered.
Enable interrupts before calling do_notify_resume(), but read
the current_thread_info->flags before doing so.
Signed-off-by: Stefan Kristiansson <stefan.kristiansson at saunalahti.fi>
---
arch/openrisc/kernel/entry.S | 5 +++--
arch/openrisc/kernel/signal.c | 7 ++++---
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index d8a455e..7564784 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
* if (current_thread_info->flags & _TIF_NEED_RESCHED)
* schedule();
*/
- l.lwz r5,TI_FLAGS(r10)
- l.andi r3,r5,_TIF_NEED_RESCHED
+ l.lwz r4,TI_FLAGS(r10)
+ l.andi r3,r4,_TIF_NEED_RESCHED
l.sfnei r3,0
l.bnf _work_notifysig
l.nop
* must be set so that the syscall restart functionality works.
*/
+ ENABLE_INTERRUPTS(r29)
l.jal do_notify_resume
l.ori r3,r1,0 /* pt_regs */
diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
index ae167f7..032bed1 100644
--- a/arch/openrisc/kernel/signal.c
+++ b/arch/openrisc/kernel/signal.c
@@ -331,12 +331,13 @@ void do_signal(struct pt_regs *regs)
return;
}
-asmlinkage void do_notify_resume(struct pt_regs *regs)
+asmlinkage void do_notify_resume(struct pt_regs *regs,
+ unsigned long thread_flags)
{
- if (current_thread_info()->flags & _TIF_SIGPENDING)
+ if (thread_flags & _TIF_SIGPENDING)
do_signal(regs);
- if (current_thread_info()->flags & _TIF_NOTIFY_RESUME) {
+ if (thread_flags & _TIF_NOTIFY_RESUME) {
clear_thread_flag(TIF_NOTIFY_RESUME);
tracehook_notify_resume(regs);
}

[Stefan Kristiansson]

Post by Jonas Bonn
Hi Stefan,
Thanks for this patch. I intend to push another patch that reworks our
signal handling code instead as it happens to have the same fix (though in
another form).
Please grab 'master' from my tree and check that it works for you. See
commit aeca89e052a618ddf9f02d55dac6684e8e0eef78.

Great, looks good.
I have to admit it was hard to follow the old work pending code when I
tried to figure out what it did, this makes it much clearer.
Good job!

Stefan

[Henrik Nordström]

Post by Stefan Kristiansson
A signal delivered through do_notify_resume() would cause the
irqs_disabled() check in _local_bh_enable_ip() to be triggered.
Enable interrupts before calling do_notify_resume().
Signed-off-by: Stefan Kristiansson <stefan.kristiansson at saunalahti.fi>

Tested-by: Henrik Nordstrom <henrik at henriknordstrom.net>

Regards
Henrik