On Wed, 5 Mar 2014 10:11:02 -0500 Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> wrote: > > On Mar 4, 2014, at 22:00, NeilBrown <neilb@xxxxxxx> wrote: > > > The access cache is used during RCU-walk path lookups, so it is best > > to avoid locking if possible as taking a lock kills concurrency. > > > > The rbtree is not rcu-safe and cannot easily be made so. > > Instead we simply check the last (i.e. most recent) entry on the LRU > > list. If this doesn't match, then we return -ECHILD and retry in > > lock/refcount mode. > > > > This requires freeing the nfs_access_entry struct with rcu, and > > requires using rcu access primates when adding entries to the lru, and > > when examining the last entry. > > > > Calling put_rpccred before kfree_rcu looks a bit odd, but as > > put_rpccred already provide rcu protection, we know that the cred will > > not actually be freed until the next grace period, so any concurrent > > access will be safe. > > > > This patch provides about 5% performance improvement on a stat-heavy > > synthetic work load with 4 threads on a 2-core CPU. > > > > Have you looked at converting the rb-tree into something a little more RCU-friendly? Since our lookup key is just a pointer value, couldn’t we use a radix-tree? > Interesting idea. The radix tree would be fairly sparse unless we divided the address by the size of the structure ... would that hit alignment issues? But we don't know real size of the structure (it can depend on auth scheme) so I doubt that would work. So I think a radix tree would waste a lot of space. Skip lists might work https://lwn.net/Articles/551896/ but they don't appear to be in the kernel yet. I think my current approach is no worse the existing code and while there is room for further improvement, that should probably wait until a suitable data structure is available. Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature
