On Wed, May 07, 2008 at 03:10:27PM -0700, Trond Myklebust wrote: > On Wed, 2008-05-07 at 14:00 -0600, Matthew Wilcox wrote: > > On Wed, May 07, 2008 at 12:44:48PM -0700, Linus Torvalds wrote: > > > On Wed, 7 May 2008, Matthew Wilcox wrote: > > > > > > > > One patch I'd still like Yanmin to test is my one from yesterday which > > > > removes the BKL from fs/locks.c. > > > > > > And I'd personally rather have the network-fs people test and comment on > > > that one ;) > > > > > > I think that patch is worth looking at regardless, but the problems with > > > that one aren't about performance, but about what the implications are for > > > the filesystems (if any)... > > > > Oh, well, they don't seem interested. > > Poor timing: we're all preparing for and travelling to the annual > Connectathon interoperability testing conference which starts tomorrow. > > > I can comment on some of the problems though. > > > > fs/lockd/svcsubs.c, fs/nfs/delegation.c, fs/nfs/nfs4state.c, > > fs/nfsd/nfs4state.c all walk the i_flock list under the BKL. That won't > > protect them against locks.c any more. That's probably OK for fs/nfs/* > > since they'll be protected by their own data structures (Someone please > > check me on that?), but it's a bad idea for lockd/nfsd which are walking > > the lists for filesystems. > > Yes. fs/nfs is just reusing the code in fs/locks.c in order to track the > locks it holds on the server. We could alternatively have coded a > private lock implementation, but this seemed easier. So, assuming nfs is taking care of its own locking (I don't know if that's right), that leaves nlm_traverse_locks() and nlm_file_inuse() (both in fs/lockd/svcsubs.c) as the problem spots. > > Are we going to have to export the file_lock_lock? I'd rather not. But > > we need to keep nfsd/lockd from tripping over locks.c. > > > > Maybe we could come up with a decent API that lockd could use? It all > > seems a bit complex at the moment ... maybe lockd should be keeping > > track of the locks it owns anyway (since surely the posix deadlock > > detection code can't work properly if it's just passing all the locks > > through). > > I'm not sure what you mean when you talk about lockd keeping track of > the locks it owns. It has to keep those locks on inode->i_flock in order > to make them visible to the host filesystem... > > All lockd really needs, is the ability to find a lock it owns, and then > obtain a copy. That sounds right. --b. > As for the nfs client, I suspect we can make do with > something similar... -- 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