[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <48936D45.2030500@goop.org>
Date: Fri, 01 Aug 2008 13:08:37 -0700
From: Jeremy Fitzhardinge <jeremy@...p.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
CC: Peter Zijlstra <a.p.zijlstra@...llo.nl>,
David Miller <davem@...emloft.net>, mingo@...e.hu,
linux-kernel@...r.kernel.org, akpm@...ux-foundation.org
Subject: Re: [PATCH] lockdep: lock_set_subclass - reset a held lock's subclass
Linus Torvalds wrote:
> Hmm.
>
> With hashed locks, that is _not_ safe in general. Now, it may well be safe
> in your case, so I'm not going to say you have a bug, but even if you do
> them in vaddr order, the thing is, we don't guarantee that the _locks_ are
> ordered in virtual address order.
>
Yes, it's current simplicity definitely relies on a simple relationship
between pte pages and locks.
> Right now, I think the pte locks are either a single one per mm (in which
> case I assume you don't take any lock at all), or it's a lock that is
> embedded in the pmd page iirc.
>
Actually the locks in the pte page, so it's already fairly
fine-grained. If it were made coarser - at the pmd level - then I'd
simply move my lock-taking out a level in the pagetable traversal.
But making it finer - by hashing individual pte entries to their own
locks - would work badly for me. I'm taking the lock specifically to
protect against updates to any pte within the pte page, so I'd have to
manually take all the locks that the ptes could possibly hash to.
Presumably in that eventuality we could define correct order for taking
the hashed pte locks, independent of how the ptes actually map to them
(for example, always take locks in low->high order of *lock* address).
> What if you have pmd sharing through some shared area being mapped at two
> different processes at different addresses? Yeah, I don't think we share
> pmd's at all (except if you use hugetables and for the kernel), but it's
> one of those things where subtle changes in how the pte lock allocation
> could cause problems.
>
In this particular case it isn't an issue. The traversal is performing
first/last use init/deinit, so any shared parts of the pagetable can be
skipped with no action because they're already done. Presumably there'd
be separate lifetime management for whatever parts of the pagetable can
be shared, so I'd need to hook in there, rather than the wholesale
pagetable create/destroy as I do now.
> Eg, I could easily see somebody doing the pte lock as a hash over not just
> the address, but the "struct mm" pointer too. At which point different
> parts of the address space might even share the PTE lock, and you'd get
> deadlocks even without any ABBA behavior, just because the pte lock might
> be A B C A or something inside the same process.
>
Yep. That would be awkward. A function which says "here's a pte page,
return the set of all pte locks these ptes map to in correct locking
order" would be useful in that case. Ugh, but still unpleasant.
J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists