From: Miklos S. <mi...@sz...> - 2007-05-25 13:37:14
|
> >> That's a good idea. I implemented it and it seems to be working here. > >> Even taking some seconds (or minutes) to complete a rename (due to the > >> access to our remote server), it was possible to keep stating the directory. > >> > > > > One more thing before I can apply it: can you please update the patch to > > the CVS version? > > > > Sure. Updated patch against the CVS version attached to the email. I am > running it here and it looks fine, but please take a quick final look at > the patch before committing. One problem I only noticed now, is that in fuse_lib_link, the patch does two separate lock_path() calls. That's a no-no, because it violates the lock ordering rules. I think we should have a lock_path2(), that can be used by both rename and link. I applied the patch and did some performance testing. I did: bunzip2 < /usr/src/linux-2.6.21.tar.gz > /dev/shm/linux-2.6.21.tar fusexmp_fh /tmp/fuse cd /tmp/fuse/dev/shm time tar xf linux-2.6.21.tar time rm -rf linux-2.6.21 ... With the patch I'm getting 10-15% slowdown on the "rm -rf". I'm not sure how much of this is the pthread code, and how much is the libfuse code, but my guess is that we may be able to do better. For example, a fairly big optimization for the uncontended case could be to try to do the locking and the path creation in a single stage, with pthread_rwlock_try*. If the trylock fails at some point we have to do it the slow way, as now. But if we can lock the whole path with trylocks we could have saved some processing. And collecting the nodes into a list isn't even needed in that case, unlocking can also be done by following the parent pointers from the original node. Also I'm a bit worried about the retry logic. Continual renames could starve any other operation being performed on that path. So for example if in one shell we have cd /a/b while true; do ls -l c; done and in another shell while true; do mv /a/b /a/d; mv /a/d /a/b; done The ->stat() for c could be starved indefinitely. Maybe we could reintroduce the original global rwlock, and rename would always take it for write (parallel renames are not allowed anyway) and other operations would take it for read only if the path verification failed. Or do you have a better idea? Miklos |