lkml.org 
[lkml]   [2014]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC 0/2] Reenable might_sleep() checks for might_fault() when atomic
> > This is what happened on our side (very recent kernel):
> >
> > spin_lock(&lock)
> > copy_to_user(...)
> > spin_unlock(&lock)
>
> That's a deadlock even without copy_to_user - it's
> enough for the thread to be preempted and another one
> to try taking the lock.
>
>
> > 1. s390 locks/unlocks a spin lock with a compare and swap, using the _cpu id_
> > as "old value"
> > 2. we slept during copy_to_user()
> > 3. the thread got scheduled onto another cpu
> > 4. spin_unlock failed as the _cpu id_ didn't match (another cpu that locked
> > the spinlock tried to unlocked it).
> > 5. lock remained locked -> deadlock
> >
> > Christian came up with the following explanation:
> > Without preemption, spin_lock() will not touch the preempt counter.
> > disable_pfault() will always touch it.
> >
> > Therefore, with preemption disabled, copy_to_user() has no idea that it is
> > running in atomic context - and will therefore try to sleep.
> >
> > So copy_to_user() will on s390:
> > 1. run "as atomic" while spin_lock() with preemption enabled.
> > 2. run "as not atomic" while spin_lock() with preemption disabled.
> > 3. run "as atomic" while pagefault_disabled() with preemption enabled or
> > disabled.
> > 4. run "as not atomic" when really not atomic.

should have been more clear at that point:
preemption enabled == kernel compiled with preemption support
preemption disabled == kernel compiled without preemption support

> >
> > And exactly nr 2. is the thing that produced the deadlock in our scenario and
> > the reason why I want a might_sleep() :)
>
> IMHO it's not copy to user that causes the problem.
> It's the misuse of spinlocks with preemption on.

As I said, preemption was off.

>
> So might_sleep would make you think copy_to_user is
> the problem, and e.g. let you paper over it by
> moving copy_to_user out.

Actually implementing different way of locking easily fixed the problem for us.
The old might_sleep() checks would have given us the problem within a few
seconds (I tested it).

>
> Enable lock prover and you will see what the real
> issue is, which is you didn't disable preempt.
> and if you did, copy_to_user would be okay.
>

Our kernel is compiled without preemption and we turned on all lock/atomic
sleep debugging aid. No problem was detected.

----
But the question is if we shouldn't rather provide a:

copy_to_user_nosleep() implementation that can be called from
pagefault_disable() because it won't sleep.
and a
copy_to_user_sleep() implementation that cannot be called from
pagefault_disable().

Another way to fix it would be a reworked pagefault_disable() function that
somehow sets "a flag", so copy_to_user() knows that it is in fact called from a
valid context, not just from "some atomic" context. So we could trigger
might_sleep() when detecting a !pagefault_disable context.



\
 
 \ /
  Last update: 2014-11-26 17:41    [W:0.252 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site