s390: in_interrupt vs. in_atomic.
PATCH 6/11 s390: in_interrupt vs. in_atomic.
[patch 6/11] s390: in_interrupt vs. in_atomic.
From: Martin Schwidefsky <[email protected]>
The condition for no context in do_exception checks for hard and soft interrupts by using in_interrupt() but not for preemption. This is bad for the users of __copy_from/to_user_inatomic because the fault handler might call schedule although the preemption count is != 0. Use in_atomic() instead in_interrupt().
diffstat:
arch/s390/mm/fault.c | 2 +- 1 files changed, 1 insertion(+), 1 deletion(-)
arch/s390/mm/fault.c +1 -1
@@ -207,7 +207,7 @@ do_exception(struct pt_regs *regs, unsig* we are not in an interrupt and that there is a * user context. */ -if (user_address ==0 ||in_interrupt () || !mm) +if (user_address ==0 ||in_atomic () || !mm) goto no_context;/*
Quoted 11 lines · Martin Schwidefsky
[patch 6/11] s390:
in_interruptvs.in_atomic.From: Martin Schwidefsky <[email protected]>
The condition for no context in
do_exceptionchecks for hard and soft interrupts by usingin_interrupt()but not for preemption. This is bad for the users of __copy_from/to_user_inatomic because the fault handler might call schedule although the preemption count is != 0. Usein_atomic()insteadin_interrupt().
hm. Under what circumstances do you expect this test to trigger?
We have the in_atomic() test in x86's do_page_fault() because as a super-special case, kmap_atomic() will increment preempt_count() even if !CONFIG_PREEMPT. This is how x86 handles faults during pagecache<->userspace copies into a kmap_atomically-mapped page. s390 doesn't do any of that.
So. What's going on in here?
arch/s390/mm/fault.c do_exception(struct pt_regs *regs, unsig · line 209
Quoted 12 lines
diffstat:
arch/s390/mm/fault.c | 2 +- 1 files changed, 1 insertion(+), 1 deletion(-)diff -urpN
linux-2.6/arch/s390/mm/fault.clinux-2.6-patched/arch/s390/mm/fault.carch/s390/mm/fault.c +0
-0@@ -207,7 +207,7 @@ do_exception(struct pt_regs *regs, unsig* we are not in an interrupt and that there is a * user context. */
Comment needs updating...
Quoted 5 lines
arch/s390/mm/fault.c +1
-1-if (user_address ==0 ||in_interrupt () || !mm) +if (user_address ==0 ||in_atomic () || !mm) goto no_context;/*
Quoted 8 lines
Quoted 6 lines
The condition for no context in
do_exceptionchecks for hard and soft interrupts by usingin_interrupt()but not for preemption. This is bad for the users of __copy_from/to_user_inatomic because the fault handler might call schedule although the preemption count is != 0. Usein_atomic()insteadin_interrupt().hm. Under what circumstances do you expect this test to trigger?
e.g. by the following:
static inline int get_futex_value_locked(int *dest, int __user *from)
{
int ret;
inc_preempt_count();
ret = __copy_from_user_inatomic(dest, from, sizeof(int));
dec_preempt_count();
preempt_check_resched();
return ret ? -EFAULT : 0;
}in_interrupt only checks for HARDIRQ_MASK and SOFTIRQ_MASK but not for the preemption counter. This is not a theory, we had a bug report concerning a "bad: scheduling while atomic!" warning.
blue skies,
Martin
Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH
Quoted 13 lines · Martin Schwidefsky
Quoted 9 lines
Quoted 7 lines
The condition for no context in
do_exceptionchecks for hard and soft interrupts by usingin_interrupt()but not for preemption. This is bad for the users of __copy_from/to_user_inatomic because the fault handler might call schedule although the preemption count is != 0. Usein_atomic()insteadin_interrupt().hm. Under what circumstances do you expect this test to trigger?
e.g. by the following:
static inline int
get_futex_value_locked(int *dest, int __user *from)
Quoted 11 lines
{
int ret;
inc_preempt_count();
ret = __copy_from_user_inatomic(dest, from, sizeof(int));
dec_preempt_count();
preempt_check_resched();
return ret ? -EFAULT : 0;
}OK, that's what it's designed for. Just checking ;)