* [PATCH linux-4.1.y] sched/wait: Fix signal handling in bit wait helpers
@ 2017-04-12 18:03 Florian Fainelli
2017-04-18 13:46 ` Oleg Nesterov
0 siblings, 1 reply; 3+ messages in thread
From: Florian Fainelli @ 2017-04-12 18:03 UTC (permalink / raw)
To: linux-kernel
Cc: stable, alexander.levin, Peter Zijlstra, Linus Torvalds,
Mike Galbraith, Thomas Gleixner, mark.rutland, neilb, oleg,
Ingo Molnar
From: Peter Zijlstra <peterz@infradead.org>
[ Upstream commit 68985633bccb6066bf1803e316fbc6c1f5b796d6 ]
Vladimir reported getting RCU stall warnings and bisected it back to
commit:
743162013d40 ("sched: Remove proliferation of wait_on_bit() action functions")
That commit inadvertently reversed the calls to schedule() and signal_pending(),
thereby not handling the case where the signal receives while we sleep.
Reported-by: Vladimir Murzin <vladimir.murzin@arm.com>
Tested-by: Vladimir Murzin <vladimir.murzin@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: mark.rutland@arm.com
Cc: neilb@suse.de
Cc: oleg@redhat.com
Fixes: 743162013d40 ("sched: Remove proliferation of wait_on_bit() action functions")
Fixes: cbbce8220949 ("SCHED: add some "wait..on_bit...timeout()" interfaces.")
Link: http://lkml.kernel.org/r/20151201130404.GL3816@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/wait.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 852143a79f36..e0bb7e6c4fb0 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -583,18 +583,18 @@ EXPORT_SYMBOL(wake_up_atomic_t);
__sched int bit_wait(struct wait_bit_key *word)
{
- if (signal_pending_state(current->state, current))
- return 1;
schedule();
+ if (signal_pending(current))
+ return -EINTR;
return 0;
}
EXPORT_SYMBOL(bit_wait);
__sched int bit_wait_io(struct wait_bit_key *word)
{
- if (signal_pending_state(current->state, current))
- return 1;
io_schedule();
+ if (signal_pending(current))
+ return -EINTR;
return 0;
}
EXPORT_SYMBOL(bit_wait_io);
@@ -602,11 +602,11 @@ EXPORT_SYMBOL(bit_wait_io);
__sched int bit_wait_timeout(struct wait_bit_key *word)
{
unsigned long now = ACCESS_ONCE(jiffies);
- if (signal_pending_state(current->state, current))
- return 1;
if (time_after_eq(now, word->timeout))
return -EAGAIN;
schedule_timeout(word->timeout - now);
+ if (signal_pending(current))
+ return -EINTR;
return 0;
}
EXPORT_SYMBOL_GPL(bit_wait_timeout);
@@ -614,11 +614,11 @@ EXPORT_SYMBOL_GPL(bit_wait_timeout);
__sched int bit_wait_io_timeout(struct wait_bit_key *word)
{
unsigned long now = ACCESS_ONCE(jiffies);
- if (signal_pending_state(current->state, current))
- return 1;
if (time_after_eq(now, word->timeout))
return -EAGAIN;
io_schedule_timeout(word->timeout - now);
+ if (signal_pending(current))
+ return -EINTR;
return 0;
}
EXPORT_SYMBOL_GPL(bit_wait_io_timeout);
--
2.12.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH linux-4.1.y] sched/wait: Fix signal handling in bit wait helpers
2017-04-12 18:03 [PATCH linux-4.1.y] sched/wait: Fix signal handling in bit wait helpers Florian Fainelli
@ 2017-04-18 13:46 ` Oleg Nesterov
2017-04-18 17:23 ` Florian Fainelli
0 siblings, 1 reply; 3+ messages in thread
From: Oleg Nesterov @ 2017-04-18 13:46 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, stable, alexander.levin, Peter Zijlstra,
Linus Torvalds, Mike Galbraith, Thomas Gleixner, mark.rutland,
neilb, Ingo Molnar
Just curious, any particular reason you want to backport this patch?
because nobody understands why it actually helped.
and please note that this patch is buggy, fixed by dfd01f0260
"sched/wait: Fix the signal handling fix"
Oleg.
On 04/12, Florian Fainelli wrote:
>
> From: Peter Zijlstra <peterz@infradead.org>
>
> [ Upstream commit 68985633bccb6066bf1803e316fbc6c1f5b796d6 ]
>
> Vladimir reported getting RCU stall warnings and bisected it back to
> commit:
>
> 743162013d40 ("sched: Remove proliferation of wait_on_bit() action functions")
>
> That commit inadvertently reversed the calls to schedule() and signal_pending(),
> thereby not handling the case where the signal receives while we sleep.
>
> Reported-by: Vladimir Murzin <vladimir.murzin@arm.com>
> Tested-by: Vladimir Murzin <vladimir.murzin@arm.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: mark.rutland@arm.com
> Cc: neilb@suse.de
> Cc: oleg@redhat.com
> Fixes: 743162013d40 ("sched: Remove proliferation of wait_on_bit() action functions")
> Fixes: cbbce8220949 ("SCHED: add some "wait..on_bit...timeout()" interfaces.")
> Link: http://lkml.kernel.org/r/20151201130404.GL3816@twins.programming.kicks-ass.net
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
> kernel/sched/wait.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
> index 852143a79f36..e0bb7e6c4fb0 100644
> --- a/kernel/sched/wait.c
> +++ b/kernel/sched/wait.c
> @@ -583,18 +583,18 @@ EXPORT_SYMBOL(wake_up_atomic_t);
>
> __sched int bit_wait(struct wait_bit_key *word)
> {
> - if (signal_pending_state(current->state, current))
> - return 1;
> schedule();
> + if (signal_pending(current))
> + return -EINTR;
> return 0;
> }
> EXPORT_SYMBOL(bit_wait);
>
> __sched int bit_wait_io(struct wait_bit_key *word)
> {
> - if (signal_pending_state(current->state, current))
> - return 1;
> io_schedule();
> + if (signal_pending(current))
> + return -EINTR;
> return 0;
> }
> EXPORT_SYMBOL(bit_wait_io);
> @@ -602,11 +602,11 @@ EXPORT_SYMBOL(bit_wait_io);
> __sched int bit_wait_timeout(struct wait_bit_key *word)
> {
> unsigned long now = ACCESS_ONCE(jiffies);
> - if (signal_pending_state(current->state, current))
> - return 1;
> if (time_after_eq(now, word->timeout))
> return -EAGAIN;
> schedule_timeout(word->timeout - now);
> + if (signal_pending(current))
> + return -EINTR;
> return 0;
> }
> EXPORT_SYMBOL_GPL(bit_wait_timeout);
> @@ -614,11 +614,11 @@ EXPORT_SYMBOL_GPL(bit_wait_timeout);
> __sched int bit_wait_io_timeout(struct wait_bit_key *word)
> {
> unsigned long now = ACCESS_ONCE(jiffies);
> - if (signal_pending_state(current->state, current))
> - return 1;
> if (time_after_eq(now, word->timeout))
> return -EAGAIN;
> io_schedule_timeout(word->timeout - now);
> + if (signal_pending(current))
> + return -EINTR;
> return 0;
> }
> EXPORT_SYMBOL_GPL(bit_wait_io_timeout);
> --
> 2.12.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH linux-4.1.y] sched/wait: Fix signal handling in bit wait helpers
2017-04-18 13:46 ` Oleg Nesterov
@ 2017-04-18 17:23 ` Florian Fainelli
0 siblings, 0 replies; 3+ messages in thread
From: Florian Fainelli @ 2017-04-18 17:23 UTC (permalink / raw)
To: Oleg Nesterov, alexander.levin
Cc: linux-kernel, stable, Peter Zijlstra, Linus Torvalds,
Mike Galbraith, Thomas Gleixner, mark.rutland, neilb,
Ingo Molnar
On 04/18/2017 06:46 AM, Oleg Nesterov wrote:
> Just curious, any particular reason you want to backport this patch?
>
> because nobody understands why it actually helped.
While working with a customer we found this patch and it did solve the
original reporters' problem and matched the issue reported by Vladimir.
>
> and please note that this patch is buggy, fixed by dfd01f0260
> "sched/wait: Fix the signal handling fix"
Ah, thanks, I missed that one!
Since Peter's fix is on top of this change, how should we proceed with
the stable-4.1 kernel? Alex, do you still actively maintain linux-4.1.y?
>
> Oleg.
>
> On 04/12, Florian Fainelli wrote:
>>
>> From: Peter Zijlstra <peterz@infradead.org>
>>
>> [ Upstream commit 68985633bccb6066bf1803e316fbc6c1f5b796d6 ]
>>
>> Vladimir reported getting RCU stall warnings and bisected it back to
>> commit:
>>
>> 743162013d40 ("sched: Remove proliferation of wait_on_bit() action functions")
>>
>> That commit inadvertently reversed the calls to schedule() and signal_pending(),
>> thereby not handling the case where the signal receives while we sleep.
>>
>> Reported-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> Tested-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Mike Galbraith <efault@gmx.de>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: mark.rutland@arm.com
>> Cc: neilb@suse.de
>> Cc: oleg@redhat.com
>> Fixes: 743162013d40 ("sched: Remove proliferation of wait_on_bit() action functions")
>> Fixes: cbbce8220949 ("SCHED: add some "wait..on_bit...timeout()" interfaces.")
>> Link: http://lkml.kernel.org/r/20151201130404.GL3816@twins.programming.kicks-ass.net
>> Signed-off-by: Ingo Molnar <mingo@kernel.org>
>> ---
>> kernel/sched/wait.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
>> index 852143a79f36..e0bb7e6c4fb0 100644
>> --- a/kernel/sched/wait.c
>> +++ b/kernel/sched/wait.c
>> @@ -583,18 +583,18 @@ EXPORT_SYMBOL(wake_up_atomic_t);
>>
>> __sched int bit_wait(struct wait_bit_key *word)
>> {
>> - if (signal_pending_state(current->state, current))
>> - return 1;
>> schedule();
>> + if (signal_pending(current))
>> + return -EINTR;
>> return 0;
>> }
>> EXPORT_SYMBOL(bit_wait);
>>
>> __sched int bit_wait_io(struct wait_bit_key *word)
>> {
>> - if (signal_pending_state(current->state, current))
>> - return 1;
>> io_schedule();
>> + if (signal_pending(current))
>> + return -EINTR;
>> return 0;
>> }
>> EXPORT_SYMBOL(bit_wait_io);
>> @@ -602,11 +602,11 @@ EXPORT_SYMBOL(bit_wait_io);
>> __sched int bit_wait_timeout(struct wait_bit_key *word)
>> {
>> unsigned long now = ACCESS_ONCE(jiffies);
>> - if (signal_pending_state(current->state, current))
>> - return 1;
>> if (time_after_eq(now, word->timeout))
>> return -EAGAIN;
>> schedule_timeout(word->timeout - now);
>> + if (signal_pending(current))
>> + return -EINTR;
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(bit_wait_timeout);
>> @@ -614,11 +614,11 @@ EXPORT_SYMBOL_GPL(bit_wait_timeout);
>> __sched int bit_wait_io_timeout(struct wait_bit_key *word)
>> {
>> unsigned long now = ACCESS_ONCE(jiffies);
>> - if (signal_pending_state(current->state, current))
>> - return 1;
>> if (time_after_eq(now, word->timeout))
>> return -EAGAIN;
>> io_schedule_timeout(word->timeout - now);
>> + if (signal_pending(current))
>> + return -EINTR;
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(bit_wait_io_timeout);
>> --
>> 2.12.1
>>
>
--
Florian
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-18 17:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-12 18:03 [PATCH linux-4.1.y] sched/wait: Fix signal handling in bit wait helpers Florian Fainelli
2017-04-18 13:46 ` Oleg Nesterov
2017-04-18 17:23 ` Florian Fainelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).