On Sun, May 27, 2007 at 12:55:30AM +0100, David Howells wrote: > J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > > > > + if (!afs_lock_manager) { > > > + afs_lock_manager = create_singlethread_workqueue("kafs_lockd"); > > > + if (!afs_lock_manager) > > > + return -ENOMEM; > > > + } > > > + return 0; > > > > Doesn't this need some locking? > > Oops. Yes. It used to be inside the lock_kernel() section, but has since > escaped. The BKL wouldn't help, since create_singlethread_workqueue() can sleep. Or am I missing something? > > Do you allow upgrades and downgrades? (Just curious.) > > AFS does not, as far as I know. So if I request a write lock while holding a read lock, my request will be denied? > > > + /* if we've already got a readlock on the server and no waiting > > > + * writelocks, then we might be able to instantly grant another > > > > Is that comment correct? (You don't really test for "waiting > > writelocks", do you?) > > Locally, yes. 'if (list_empty(&vnode->pending_locks))' covers it quite > handily. Oops, right, I was overlooking that check. This is a little strange, though--if there's somebody waiting for a write lock on an inode (because somebody else already holds a read lock on it), that shouldn't block requests for read locks. --b. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html