On Wed, 16 Oct 2013 22:52:07 +0200 Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > Hey Neil; > > it looks like its one of your patches isn't it? > > http://www.spinics.net/lists/raid/msg44100.html > http://www.spinics.net/lists/raid/msg44101.html > > Given that I can't find them in a lkml archive means nobody's ever seen > those patches. > > Anyway; has that 3/3 patch ever been ran with lockdep enabled? I always run with lockdep enabled, and I have done at least basic testing (I've been on leave for a few weeks and don't remember exactly where I got to). And I haven't seen any lockdep reports. > > Stuff like: > > + for (i = 0; i < NR_STRIPE_HASH_LOCKS; i++) > + spin_lock_init(conf->hash_locks + i); > > And: > > +static void __lock_all_hash_locks(struct r5conf *conf) > +{ > + int i; > + for (i = 0; i < NR_STRIPE_HASH_LOCKS; i++) > + spin_lock(conf->hash_locks + i); > +} > > Tends to complain real loud. Why is that? Because "conf->hash_locks + i" gets used as the "name" of the lockdep map for each one, and when they are all locked it looks like nested locking?? the lock_all_hash_locks doesn't get called very often and my testing mustn't have got that far. I just tried something that would trigger the "lock_all_device_hash_locks_irq" (as it is in the current version) and it went "splat" just as you said it would. Thanks. Do you have a suggestion for how to make this work? Would spin_lock_nested(conf->hash_locks + i, i) do the trick? Thanks, NeilBrown > > This leaves one to wonder... > 'fancy' locking scheme:1, validation effort:0 >
Attachment:
signature.asc
Description: PGP signature
