Messages in this thread | | | Date | Thu, 29 Aug 2013 17:49:46 -0700 | Subject | Re: [PATCH v7 1/4] spinlock: A new lockref structure for lockless update of refcount | From | Linus Torvalds <> |
| |
On Thu, Aug 29, 2013 at 5:26 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > I assume you mean unsigned int ? :-)
Oops, yes.
> What's wrong with the existing arch_spin_is_locked() ?
It takes a memory location. And we very much want to test the value we loaded into a register.
And yes, gcc can do the right thing. But at least on x86, arch_spin_is_locked() actually uses ACCESS_ONCE() to load the value from the memory location, and I actually think that is the right thing to do (or at least not incorrect). So the end result is that arch_spin_value_unlocked() is actually fairly fundamentally different from arch_spin_is_locked().
So I could have re-used arch_spin_is_locked() after having changed the semantics of it, but I really didn't want to possibly change totally unrelated users for this particular feature.
> BTW. Do you have your test case at hand ?
My test-case is a joke. It's explicitly *trying* to get as much contention as possible on a dentry, by just starting up a lot of threads that look up one single pathname (the same one for everybody). It defaults to using /tmp for this, but you can specify the filename.
Note that directories, regular files and symlinks have fundamentally different dentry lookup behavior:
- directories tend to have an elevated reference count (because they have children). This was my primary test-case, because while I suspect that there are crazy loads (and AIM7 may be one of them) that open the same _regular_ file all concurrently, I don't think it's a "normal" load). But opening the same directory concurrently as part of pathname lookup is certainly normal.
- regular files tend to have a dentry count of zero unless they are actively open, and the patch I sent out will take the dentry spinlock for them when doing the final RCU finishing touches if that's the case. So this one *will* still use the per-dentry spinlock rather than the lockless refcount increments, but as outlined above I don't think that should be a scalability issue unless you're crazy.
- symlink traveral causes us to drop out of RCU lookup mode, and thus cause various slow-paths to happen. Some of that we can improve on, but I suspect it will cause the lockless refcount paths to take a hit too.
Anyway, I'm attaching my completely mindless test program. It has hacky things like "unsigned long count[MAXTHREADS][32]" which are purely to just spread out the counts so that they aren't in the same cacheline etc.
Also note that the performance numbers it spits out depend a lot on tings like how long the dcache hash chains etc are, so they are not really reliable. Running the test-program right after reboot when the dentries haven't been populated can result in much higher numbers - without that having anything to do with contention or locking at all.
Linus #include <stdio.h> #include <stdlib.h> #include <pthread.h> #include <sys/types.h> #include <sys/stat.h> #include <unistd.h>
#define MAXTHREADS 16
static volatile int start = 0; static char *file = "/tmp"; static unsigned long count[MAXTHREADS][32];
void *start_routine(void *arg) { const char *filename; struct stat st; unsigned long *counter = arg;
pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL); while (!start) /* nothing */; filename = file; for (;;) { stat(filename, &st); ++*counter; } }
int main(int argc, char **argv) { pthread_t threads[MAXTHREADS]; unsigned long n; int i;
if (argv[1]) file = argv[1]; for (i = 0; i < MAXTHREADS; i++) pthread_create(threads+i, NULL, start_routine, count[i]); start = 1; sleep(10); for (i = 0; i < MAXTHREADS; i++) pthread_cancel(threads[i]); for (i = 0; i < MAXTHREADS; i++) pthread_join(threads[i], NULL); n = 0; for (i = 0; i < MAXTHREADS; i++) n += count[i][0]; printf("Total loops: %lu\n", n); return 0; }
| |