On Mon, 2 Jun 2014 11:20:26 -0700 Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > On Mon, Jun 02, 2014 at 10:17:33AM -0400, Jeff Layton wrote: > > Ok, fair enough. A later patch in the series adds a per nfs4_file lock > > (the fi_lock) that we can use for this purpose in lieu of the i_lock. > > Nesting that within the i_lock shouldn't be too painful. > > i_lock is part of some very complex locking hierachies in the inode > and dentry caches. Long term I'd prefer to get the file locking code > detangled from that, but let's keep it simple for now and nest > the nfs4_file lock inside for now. > > > We could do that within the rpc_call_prepare operation, but the main > > problem there is that the delegation could leak if the rpc task > > allocation fails. Would you be amenable to adding a "cb_prepare" > > operation or something that we could use to run things from the > > workqueue before the actual callback runs? > > I guess we'll have to do it then. I'll see if it can be done nicely. > There is another option, but it's sort of ugly... We could try to queue the thing in the rpc_call_prepare op, but if that fails to occur then we could do it in the rpc_release callback. The tricky part is figuring out whether we'll need to queue it in rpc_release. Perhaps we can try to use the dl_time field to indicate that as well so we won't need to add a separate flag for it. > I suspect the root problem of all this is that the file locking code > calls the lock manager ops with i_lock held, which isn't very nice. > Yeah, it would be nice if it didn't, but it's better than it was. It used to use a global spinlock for that instead of the i_lock, but that was also a clear scalability problem... -- Jeff Layton <jlayton@xxxxxxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
