On Sat, Jun 02, 2007 at 02:21:22PM -0400, Trond Myklebust wrote: > Currently, the lease handling is done all in the VFS, and is done prior > to calling any filesystem operations. Bruce's break_lease() inode > operation allows the VFS to notify the filesystem that some operation is > going to be called that requires the lease to be broken. > > My point is that in doing so, you are not atomic with the operation that > requires the lease to be broken. Some different node may re-establish a > lease while we're calling back down into the filesystem to perform the > operation. > So I agree with you. The break_lease() inode operation isn't going to > work. The filesystem is going to have to figure out for itself when it > needs to notify other nodes that the lease needs breaking, and it needs > to figure out its own methods for ensuring atomicity. OK, I agree with you both, thanks for the explanations. It looks to me like there's probably a race in the existing code that will allow conflicting opens and leases to be granted simultaneously if the lease request is handled just after may_open() is called. These checks at the beginning of __setlease() are an attempt to prevent that race: if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0)) goto out; if ((arg == F_WRLCK) && ((atomic_read(&dentry->d_count) > 1) || (atomic_read(&inode->i_count) > 1))) goto out; But, for example, in the case of a simultaneous write open and RDLCK lease request, I believe the call to setlease could come after the may_open() but before the call to get_write_access() that bumps i_writecount. --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
