2006-12-14 23:04:46

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/10] lockd: add new export operation for nfsv4/lockd locking

By the way, one other issue I think we'll need to resolve:

On Wed, Dec 06, 2006 at 12:34:11AM -0500, J. Bruce Fields wrote:
> +/**
> + * vfs_cancel_lock - file byte range unblock lock
> + * @filp: The file to apply the unblock to
> + * @fl: The lock to be unblocked
> + *
> + * FL_CANCELED is used to cancel blocked requests
> + */
> +int vfs_cancel_lock(struct file *filp, struct file_lock *fl)
> +{
> + int status;
> + struct super_block *sb;
> +
> + fl->fl_flags |= FL_CANCEL;
> + sb = filp->f_dentry->d_inode->i_sb;
> + if (sb->s_export_op && sb->s_export_op->lock)
> + status = sb->s_export_op->lock(filp, F_SETLK, fl);
> + else
> + status = posix_unblock_lock(filp, fl);
> + fl->fl_flags &= ~FL_CANCEL;
> + return status;
> +}

So we're passing cancel requests to the filesystem by setting an
FL_CANCEL flag in fl_flags and then calling the lock operation. I think
Trond has said he'd rather keep fl_flags for permanent characteristics
of the lock in question, rather than as a channel for passing arguments
to lock operations. Also, the GFS patch isn't checking FL_CANCEL, so
that's a bug.

--b.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2006-12-15 05:52:47

by Marc Eshel

[permalink] [raw]
Subject: Re: [PATCH 1/10] lockd: add new export operation for nfsv4/lockd locking

Lets see if we use export operations or vfs calls. If we do exports we can
even add another call just for cancel or maybe we can add a new vfs call.
Marc.

"J. Bruce Fields" <[email protected]> wrote on 12/14/2006 03:04:42 PM:

> By the way, one other issue I think we'll need to resolve:
>
> On Wed, Dec 06, 2006 at 12:34:11AM -0500, J. Bruce Fields wrote:
> > +/**
> > + * vfs_cancel_lock - file byte range unblock lock
> > + * @filp: The file to apply the unblock to
> > + * @fl: The lock to be unblocked
> > + *
> > + * FL_CANCELED is used to cancel blocked requests
> > + */
> > +int vfs_cancel_lock(struct file *filp, struct file_lock *fl)
> > +{
> > + int status;
> > + struct super_block *sb;
> > +
> > + fl->fl_flags |= FL_CANCEL;
> > + sb = filp->f_dentry->d_inode->i_sb;
> > + if (sb->s_export_op && sb->s_export_op->lock)
> > + status = sb->s_export_op->lock(filp, F_SETLK, fl);
> > + else
> > + status = posix_unblock_lock(filp, fl);
> > + fl->fl_flags &= ~FL_CANCEL;
> > + return status;
> > +}
>
> So we're passing cancel requests to the filesystem by setting an
> FL_CANCEL flag in fl_flags and then calling the lock operation. I think
> Trond has said he'd rather keep fl_flags for permanent characteristics
> of the lock in question, rather than as a channel for passing arguments
> to lock operations. Also, the GFS patch isn't checking FL_CANCEL, so
> that's a bug.
>
> --b.


2006-12-15 07:40:43

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/10] lockd: add new export operation for nfsv4/lockd locking

On Thu, Dec 14, 2006 at 06:04:42PM -0500, J. Bruce Fields wrote:
> By the way, one other issue I think we'll need to resolve:
>
> On Wed, Dec 06, 2006 at 12:34:11AM -0500, J. Bruce Fields wrote:
> > +/**
> > + * vfs_cancel_lock - file byte range unblock lock
> > + * @filp: The file to apply the unblock to
> > + * @fl: The lock to be unblocked
> > + *
> > + * FL_CANCELED is used to cancel blocked requests
> > + */
> > +int vfs_cancel_lock(struct file *filp, struct file_lock *fl)
> > +{
> > + int status;
> > + struct super_block *sb;
> > +
> > + fl->fl_flags |= FL_CANCEL;
> > + sb = filp->f_dentry->d_inode->i_sb;
> > + if (sb->s_export_op && sb->s_export_op->lock)
> > + status = sb->s_export_op->lock(filp, F_SETLK, fl);
> > + else
> > + status = posix_unblock_lock(filp, fl);
> > + fl->fl_flags &= ~FL_CANCEL;
> > + return status;
> > +}
>
> So we're passing cancel requests to the filesystem by setting an
> FL_CANCEL flag in fl_flags and then calling the lock operation. I think
> Trond has said he'd rather keep fl_flags for permanent characteristics
> of the lock in question, rather than as a channel for passing arguments
> to lock operations. Also, the GFS patch isn't checking FL_CANCEL, so
> that's a bug.

This should be a separate operation. Either a new command for
->lock or a completely new operation.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs