From: john stultz on 22 Jun 2010 02:10 On Mon, 2010-06-21 at 16:55 +0200, Peter Zijlstra wrote: > On Tue, 2010-06-22 at 00:48 +1000, Nick Piggin wrote: > > > Right, so I was staring at the -rt splat, so its John who created that > > > wreckage? > > > > It was, but apparently they saw an RCU bug there somewhere and hit it > > with the big hammer. I haven't been able to reproduce it on a non-rt > > kernel yet, and I see yet why RCU is not good enough here. > > John, could you describe the failure you spotted? The problem was that the rcu_read_lock() on the dentry ascending wasn't preventing d_put/d_kill from removing entries from the parent node. So the next entry we tried to follow was invalid. So we were getting odd oopses from select_parent(). I'm not as familiar with the rcu rules there, so the patch I made just held the locks as it went down the chain. Not ideal of course, but still an improvement over the dcache_lock that was there prior. Peter: I'm sorry, I've been out for a few days. Can you give me some background on what brought this up and what -rt splat you mean? thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Nick Piggin on 22 Jun 2010 02:10 On Mon, Jun 21, 2010 at 11:02:37PM -0700, John Stultz wrote: > On Mon, 2010-06-21 at 16:55 +0200, Peter Zijlstra wrote: > > On Tue, 2010-06-22 at 00:48 +1000, Nick Piggin wrote: > > > > Right, so I was staring at the -rt splat, so its John who created that > > > > wreckage? > > > > > > It was, but apparently they saw an RCU bug there somewhere and hit it > > > with the big hammer. I haven't been able to reproduce it on a non-rt > > > kernel yet, and I see yet why RCU is not good enough here. > > > > John, could you describe the failure you spotted? > > The problem was that the rcu_read_lock() on the dentry ascending wasn't > preventing d_put/d_kill from removing entries from the parent node. So > the next entry we tried to follow was invalid. So we were getting odd > oopses from select_parent(). Oh, ah OK that makes sense. I was thinking it was a use after grace period problem. Hmm, I'll think about whether we can fix it better. > > I'm not as familiar with the rcu rules there, so the patch I made just > held the locks as it went down the chain. Not ideal of course, but still > an improvement over the dcache_lock that was there prior. > > Peter: I'm sorry, I've been out for a few days. Can you give me some > background on what brought this up and what -rt splat you mean? > > thanks > -john > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on 22 Jun 2010 03:30 On Mon, 2010-06-21 at 23:02 -0700, john stultz wrote: > On Mon, 2010-06-21 at 16:55 +0200, Peter Zijlstra wrote: > > On Tue, 2010-06-22 at 00:48 +1000, Nick Piggin wrote: > > > > Right, so I was staring at the -rt splat, so its John who created that > > > > wreckage? > > > > > > It was, but apparently they saw an RCU bug there somewhere and hit it > > > with the big hammer. I haven't been able to reproduce it on a non-rt > > > kernel yet, and I see yet why RCU is not good enough here. > > > > John, could you describe the failure you spotted? > > The problem was that the rcu_read_lock() on the dentry ascending wasn't > preventing d_put/d_kill from removing entries from the parent node. So > the next entry we tried to follow was invalid. So we were getting odd > oopses from select_parent(). > > I'm not as familiar with the rcu rules there, so the patch I made just > held the locks as it went down the chain. Not ideal of course, but still > an improvement over the dcache_lock that was there prior. > > Peter: I'm sorry, I've been out for a few days. Can you give me some > background on what brought this up and what -rt splat you mean? Well, you make lockdep very unhappy by locking multiple dentries (unbounded number) all in the same lock class. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: john stultz on 22 Jun 2010 22:10 On Tue, 2010-06-22 at 09:27 +0200, Peter Zijlstra wrote: > On Mon, 2010-06-21 at 23:02 -0700, john stultz wrote: > > On Mon, 2010-06-21 at 16:55 +0200, Peter Zijlstra wrote: > > > On Tue, 2010-06-22 at 00:48 +1000, Nick Piggin wrote: > > > > > Right, so I was staring at the -rt splat, so its John who created that > > > > > wreckage? > > > > > > > > It was, but apparently they saw an RCU bug there somewhere and hit it > > > > with the big hammer. I haven't been able to reproduce it on a non-rt > > > > kernel yet, and I see yet why RCU is not good enough here. > > > > > > John, could you describe the failure you spotted? > > > > The problem was that the rcu_read_lock() on the dentry ascending wasn't > > preventing d_put/d_kill from removing entries from the parent node. So > > the next entry we tried to follow was invalid. So we were getting odd > > oopses from select_parent(). > > > > I'm not as familiar with the rcu rules there, so the patch I made just > > held the locks as it went down the chain. Not ideal of course, but still > > an improvement over the dcache_lock that was there prior. > > > > Peter: I'm sorry, I've been out for a few days. Can you give me some > > background on what brought this up and what -rt splat you mean? > > Well, you make lockdep very unhappy by locking multiple dentries > (unbounded number) all in the same lock class. So.. Is there a way to tell lockdep that the nesting is ok (I thought that was what the spin_lock_nested call was doing...)? Or is locking a (possibly quite long) chain of objects really just a do-not-do type of operation? thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on 23 Jun 2010 03:30
On Tue, 2010-06-22 at 19:03 -0700, john stultz wrote: > > Well, you make lockdep very unhappy by locking multiple dentries > > (unbounded number) all in the same lock class. > > So.. Is there a way to tell lockdep that the nesting is ok (I thought > that was what the spin_lock_nested call was doing...)? spin_lock_nested() allows you to nest a limited number of locks (up to 8, although the usual case is 1). > Or is locking a (possibly quite long) chain of objects really just a > do-not-do type of operation? Usually, yeah. It would be really nice to do this another way (also for scalability, keeping a large subtree locked is bound to to lead to more contention). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |