On Mon, Jun 13, 2011 at 08:19:39AM -0400, J. Bruce Fields wrote: > On Sun, Jun 12, 2011 at 04:54:33PM -0400, Mimi Zohar wrote: > > You're adding a call to break_lease() for each of them. Currently > > __break_lease() is only called if a lease exists. Assuming there aren't > > any existing leases, couldn't break_lease() call something like > > block_lease()? The free would be after the link, unlink, ..., > > completed/failed. > > > > (You wouldn't actually need to alloc/free the 'struct file_lock' each > > time, just set the pointer and reset to NULL.) > > Well, the pointer has to be set to something. I suppose we could put a > struct file_lock on the stack. The locking code is under a global spinlock--instead of an atomic inc and dec of inode->i_blockleases we'd be doing a pair of lock/unlocks of file_lock_lock. We could probably fix that: off the top of my head the only reason I see for a global lock is the stupid deadlock-detection code. Which is only needed for posix locks (and I'm not at all convinced it's needed even there). With that fixed, it would be a choice between an i_lock-protected list_add/list_del vs. an atomic inc/dec of a new inode field. --b. -- 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