Am 26.11.2014 um 16:47 schrieb Michael S. Tsirkin: > On Wed, Nov 26, 2014 at 04:32:07PM +0100, David Hildenbrand wrote: >>> On Wed, Nov 26, 2014 at 05:17:29PM +0200, Michael S. Tsirkin wrote: >>>> On Wed, Nov 26, 2014 at 11:05:04AM +0100, David Hildenbrand wrote: >>>>>> What's the path you are trying to debug? >>>>> >>>>> Well, we had a problem where we held a spin_lock and called >>>>> copy_(from|to)_user(). We experienced very random deadlocks that took some guy >>>>> almost a week to debug. The simple might_sleep() check would have showed this >>>>> error immediately. >>>> >>>> This must have been a very old kernel. >>>> A modern kernel will return an error from copy_to_user. >>>> Which is really the point of the patch you are trying to revert. >>> >>> That's assuming you disabled preemption. If you didn't, and take >>> a spinlock, you have deadlocks even without userspace access. >>> >> >> (Thanks for your resent, my first email was sent directly to you ... grml) >> >> 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. Huh? With CONFIG_PREEMPT spin_lock will disable preemption. (we had preempt = server anyway). But please: One step back. The problem is not the good path. The problem is that we lost a debugging aid for a known to be broken case. In other words: Our code had a bug. Older kernels detected that kind of bug. With your change we no longer saw the sleeping while atomic. Thats it. See my other mail. Christian -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
