Re: [PATCH] locks: provide a file lease method enabling cluster-coherent leases

["Robert Rappaport" <robert.rappaport@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2723 bytes --]

I have had some previous communications with Bruce on these topics, and I am
generally pleased with the proposed modifications that are presented here.

I am working on a clustered file system and there is one small additional
modification that would be of great use to me.  That would be to export the
__setlease symbol.

In my implementation, my file system specific set_lease() function first
determines whether the granting of the requested lease on the given inode is
compatible with the cluster state of this inode, and then if it is, I simply
invoke the __setlease() routine and have the kernel build the associated
infrastructure.

Having this symbol globally available greatly simplifies things.

On 6/4/07, J. Bruce Fields <bfields@fieldses.org> wrote:
>
> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[-- Attachment #1.2: Type: text/html, Size: 3691 bytes --]

[-- Attachment #2: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #3: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs