Re: [PATCH 1/2] locks: introduce i_blockleases to close lease races

["J. Bruce Fields" <bfields@fieldses.org>]

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.