[PATCH] do_work_pending: Enable interrupts in do_work_pending()

[PATCH] do_work_pending: Enable interrupts in do_work_pending()

[Anand Moon]

A recent change triggered a WARN_ON that interrupts were disabled,
and in fact other architectures enable interrupts uniformly for their
related do_work_pending() type code, and all the things we call from
this routine appear to expect interrupts to be enabled.  So, enable them.
Below is the warning.

[   12.094106] systemd-udevd[1643]: starting version 204
[   13.466537] ------------[ cut here ]------------
[   13.469750] WARNING: CPU: 1 PID: 1645 at kernel/locking/lockdep.c:3536 check_flags+0x7c/0x1d0()
[   13.478397] DEBUG_LOCKS_WARN_ON(current->hardirqs_enabled)
[   13.483663] Modules linked in:
[   13.486863] CPU: 1 PID: 1645 Comm: systemd-udevd Not tainted 3.17.4-arm7 #6
[   13.493851] [<c0013dc8>] (unwind_backtrace) from [<c0011208>] (show_stack+0x10/0x14)
[   13.501563] [<c0011208>] (show_stack) from [<c05df5f0>] (dump_stack+0x80/0xac)
[   13.508753] [<c05df5f0>] (dump_stack) from [<c0024594>] (warn_slowpath_common+0x60/0x84)
[   13.516808] [<c0024594>] (warn_slowpath_common) from [<c00245e4>] (warn_slowpath_fmt+0x2c/0x3c)
[   13.525463] [<c00245e4>] (warn_slowpath_fmt) from [<c006c508>] (check_flags+0x7c/0x1d0)
[   13.533447] [<c006c508>] (check_flags) from [<c006cf60>] (lock_is_held+0x34/0x60)
[   13.540884] [<c006cf60>] (lock_is_held) from [<c05e0500>] (__schedule+0x98/0x7cc)
[   13.548342] [<c05e0500>] (__schedule) from [<c0010adc>] (do_work_pending+0x28/0xbc)
[   13.555962] [<c0010adc>] (do_work_pending) from [<c000dd38>] (work_pending+0xc/0x20)
[   13.563663] ---[ end trace 33bbfa4173cfa520 ]---
[   13.568238] possible reason: unannotated irqs-off.
[   13.573005] irq event stamp: 5227
[   13.576293] hardirqs last  enabled at (5227): [<c000dd04>] ret_fast_syscall+0x24/0x48
[   13.584096] hardirqs last disabled at (5226): [<c000dcec>] ret_fast_syscall+0xc/0x48
[   13.591823] softirqs last  enabled at (5222): [<c00283b0>] __do_softirq+0x1e8/0x270
[   13.599447] softirqs last disabled at (5205): [<c00286f4>] irq_exit+0x84/0xf4
[   13.656634] random: nonblocking pool is initialized

Signed-off-by: Anand Moon <moon.linux@yahoo.com>
---
 arch/arm/kernel/signal.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index bd19834..52bc9d6 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -476,7 +476,7 @@ setup_rt_frame(struct ksignal *ksig, sigset_t *set, struct pt_regs *regs)
 
 /*
  * OK, we're invoking a handler
- */	
+ */
 static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 {
 	sigset_t *oldset = sigmask_to_save();
@@ -574,12 +574,14 @@ asmlinkage int
 do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
 {
 	do {
+		if (unlikely(!user_mode(regs)))
+			return 0;
+		/* Enable interrupts; they are disabled again on return to
+		 * caller. */
+		local_irq_enable();
 		if (likely(thread_flags & _TIF_NEED_RESCHED)) {
 			schedule();
 		} else {
-			if (unlikely(!user_mode(regs)))
-				return 0;
-			local_irq_enable();
 			if (thread_flags & _TIF_SIGPENDING) {
 				int restart = do_signal(regs, syscall);
 				if (unlikely(restart)) {
@@ -588,6 +590,7 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
 					 * Deal with it without leaving
 					 * the kernel space.
 					 */
+					local_irq_disable();
 					return restart;
 				}
 				syscall = 0;
-- 
2.1.0

[PATCH] do_work_pending: Enable interrupts in do_work_pending()

[Russell King - ARM Linux]

On Fri, Dec 05, 2014 at 08:21:06PM +0530, Anand Moon wrote:
> @@ -574,12 +574,14 @@ asmlinkage int
>  do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
>  {
>  	do {
> +		if (unlikely(!user_mode(regs)))
> +			return 0;
> +		/* Enable interrupts; they are disabled again on return to
> +		 * caller. */
> +		local_irq_enable();
>  		if (likely(thread_flags & _TIF_NEED_RESCHED)) {
>  			schedule();
>  		} else {
> -			if (unlikely(!user_mode(regs)))
> -				return 0;
> -			local_irq_enable();
>  			if (thread_flags & _TIF_SIGPENDING) {
>  				int restart = do_signal(regs, syscall);
>  				if (unlikely(restart)) {

I'm happy with the hunk above, but:

> @@ -588,6 +590,7 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
>  					 * Deal with it without leaving
>  					 * the kernel space.
>  					 */
> +					local_irq_disable();
>  					return restart;

not this one.  The code expects in the non-zero return case, that
interrupts will be enabled, otherwise we will be restarting the syscall
with IRQs disabled, and calling into the syscall function with IRQs
disabled.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCH] do_work_pending: Enable interrupts in do_work_pending()

[Anand Moon]

hi Russell King,

I was experiencing the same problem. You mention.
I realized later after few application failure an system slowdown.

-Anand Moon

On 12/5/2014 8:43 PM, Russell King - ARM Linux wrote:
> On Fri, Dec 05, 2014 at 08:21:06PM +0530, Anand Moon wrote:
>> @@ -574,12 +574,14 @@ asmlinkage int
>>   do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
>>   {
>>   	do {
>> +		if (unlikely(!user_mode(regs)))
>> +			return 0;
>> +		/* Enable interrupts; they are disabled again on return to
>> +		 * caller. */
>> +		local_irq_enable();
>>   		if (likely(thread_flags & _TIF_NEED_RESCHED)) {
>>   			schedule();
>>   		} else {
>> -			if (unlikely(!user_mode(regs)))
>> -				return 0;
>> -			local_irq_enable();
>>   			if (thread_flags & _TIF_SIGPENDING) {
>>   				int restart = do_signal(regs, syscall);
>>   				if (unlikely(restart)) {
> I'm happy with the hunk above, but:
>
>> @@ -588,6 +590,7 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
>>   					 * Deal with it without leaving
>>   					 * the kernel space.
>>   					 */
>> +					local_irq_disable();
>>   					return restart;
> not this one.  The code expects in the non-zero return case, that
> interrupts will be enabled, otherwise we will be restarting the syscall
> with IRQs disabled, and calling into the syscall function with IRQs
> disabled.
>