Include OFD lock owners when looking up state

PATCH Superseded Benjamin Coddington 5 participants 16 messages Apr 1, 2016

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Benjamin Coddington

author

2016-04-01 15:34 UTC #0 lore raw

cover Include OFD lock owners when looking up state

The client sends IO only under the open stateid when using OFD (and flock) locking instead of the appropriate lock stateid because the nfs_lock_context only tracks POSIX lockowners, which is the reference to the process' file table.

This is a problem for two reasons. The first is that rfc7530, section-9.1.4.5 states that IO sent by an entity corresponding to the lock-owner which holds a byte-range lock should be sent under the lock stateid for that lock. Otherwise, a server enforcing mandatory byte-range locking might reject that operation. Secondly, not tracking OFD lock owners means that accounting for IO sent under those owners is broken. That creates a problem for some work to guarantee an unlock will be sent after operations scheduled under a lock complete.

To fix this, this set starts tracking the OFD lock owner within the nfs_lock_context. Unique nfs_lock_contexts are created on distinct combinations of current->files, struct file, and pid. In order to do this, some re-arrangement of when to create or lookup the nfs_lock_context was needed since the struct file pointer must be included during that creation or lookup.

It is possible for a process to hold both an OFD and a POSIX lock on a file as long as they do not conflict. There would be two stateids that could be selected for IO operations on that file. In that case only the first stateid found to match the current lock context will ever be used. The result of this is that mixed OFD and POSIX locks within the same process on a server enforcing mandatory locking should be avoided. The fix for this probably would require a byte-range lookup of stateid when scheduling an IO operation, and the ability to split IO that crossed lock ranges with different owners.

Comments and critique is welcomed.

Benjamin Coddington (3):
NFS: add get_nfs_lock_context, find_nfs_lock_context
NFS: add OFD lock owners to nfs_lock_context
NFSv4: use OFD lock owners in lock state lookup

 fs/nfs/direct.c          |    8 ++++----
 fs/nfs/file.c            |    2 +-
 fs/nfs/fscache-index.c   |    4 ++--
 fs/nfs/fscache.h         |   12 ++++++------
 fs/nfs/inode.c           |   32 ++++++++++++++++++++++----------
 fs/nfs/nfs42proc.c       |    8 ++++----
 fs/nfs/nfs4proc.c        |    3 ++-
 fs/nfs/nfs4state.c       |   21 ++++++++++++---------
 fs/nfs/pagelist.c        |   22 ++++++++--------------
 fs/nfs/read.c            |   38 +++++++++++++++++++++++---------------
 fs/nfs/write.c           |   17 ++++++++++-------
 include/linux/nfs_fs.h   |    8 +++++---
 include/linux/nfs_page.h |    2 +-
 13 files changed, 100 insertions(+), 77 deletions(-)

Benjamin Coddington

author

2016-04-01 15:34 UTC #1 lore raw

PATCH 1/3 NFS: add get_nfs_lock_context, find_nfs_lock_context

Create a method for acquiring a nfs_lock_context reference which will be used later within fscache, and rename nfs_get_lock_context to nfs_find_lock_context to prepare it for finding or allocating a lock context based upon a file pointer.

Signed-off-by Benjamin Coddington <[email protected]>

---
 fs/nfs/direct.c        |    4 ++--
 fs/nfs/file.c          |    2 +-
 fs/nfs/inode.c         |   11 +++++++++--
 fs/nfs/nfs42proc.c     |    8 ++++----
 fs/nfs/pagelist.c      |    2 +-
 include/linux/nfs_fs.h |    3 ++-
 6 files changed, 19 insertions(+), 11 deletions(-)

fs/nfs/direct.c +2 -2

index 7a0cfd3..5f833fa 100644
@@ -596,7 +596,7 @@ ssize_t nfs_file_direct_read(struct kiocb *iocb, struct iov_iter *iter,
 	dreq->bytes_left = count;
 	dreq->io_start = pos;
 	dreq->ctx = get_nfs_open_context(nfs_file_open_context(iocb->ki_filp));
-	l_ctx = nfs_get_lock_context(dreq->ctx);
+	l_ctx = nfs_find_lock_context(dreq->ctx);
 	if (IS_ERR(l_ctx)) {
 		result = PTR_ERR(l_ctx);
 		goto out_release;

@@ -1029,7 +1029,7 @@ ssize_t nfs_file_direct_write(struct kiocb *iocb, struct iov_iter *iter)
 	dreq->bytes_left = iov_iter_count(iter);
 	dreq->io_start = pos;
 	dreq->ctx = get_nfs_open_context(nfs_file_open_context(iocb->ki_filp));
-	l_ctx = nfs_get_lock_context(dreq->ctx);
+	l_ctx = nfs_find_lock_context(dreq->ctx);
 	if (IS_ERR(l_ctx)) {
 		result = PTR_ERR(l_ctx);
 		goto out_release;

fs/nfs/file.c +1 -1

index 748bb81..f99c085 100644
@@ -754,7 +754,7 @@ do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 	 */
 	vfs_fsync(filp, 0);
 
-	l_ctx = nfs_get_lock_context(nfs_file_open_context(filp));
+	l_ctx = nfs_find_lock_context(nfs_file_open_context(filp));
 	if (!IS_ERR(l_ctx)) {
 		status = nfs_iocounter_wait(l_ctx);
 		nfs_put_lock_context(l_ctx);

fs/nfs/inode.c +9 -2

index 86faecf..5d484e5 100644
@@ -725,7 +725,7 @@ static struct nfs_lock_context *__nfs_find_lock_context(struct nfs_open_context
 	return NULL;
 }
 
-struct nfs_lock_context *nfs_get_lock_context(struct nfs_open_context *ctx)
+struct nfs_lock_context *nfs_find_lock_context(struct nfs_open_context *ctx)
 {
 	struct nfs_lock_context *res, *new = NULL;
 	struct inode *inode = d_inode(ctx->dentry);

@@ -751,7 +751,14 @@ struct nfs_lock_context *nfs_get_lock_context(struct nfs_open_context *ctx)
 	kfree(new);
 	return res;
 }
-EXPORT_SYMBOL_GPL(nfs_get_lock_context);
+EXPORT_SYMBOL_GPL(nfs_find_lock_context);
+
+struct nfs_lock_context *get_nfs_lock_context(struct nfs_lock_context *l_ctx)
+{
+	atomic_inc(&l_ctx->count);
+	return l_ctx;
+}
+EXPORT_SYMBOL_GPL(get_nfs_lock_context);
 
 void nfs_put_lock_context(struct nfs_lock_context *l_ctx)
 {

fs/nfs/nfs42proc.c +4 -4

index dff8346..f9b0f25 100644
@@ -61,7 +61,7 @@ static int nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
 	struct nfs_lock_context *lock;
 	int err;
 
-	lock = nfs_get_lock_context(nfs_file_open_context(filep));
+	lock = nfs_find_lock_context(nfs_file_open_context(filep));
 	if (IS_ERR(lock))
 		return PTR_ERR(lock);

@@ -171,7 +171,7 @@ loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
 	struct nfs_lock_context *lock;
 	loff_t err;
 
-	lock = nfs_get_lock_context(nfs_file_open_context(filep));
+	lock = nfs_find_lock_context(nfs_file_open_context(filep));
 	if (IS_ERR(lock))
 		return PTR_ERR(lock);

@@ -365,14 +365,14 @@ int nfs42_proc_clone(struct file *src_f, struct file *dst_f,
 	if (!nfs_server_capable(inode, NFS_CAP_CLONE))
 		return -EOPNOTSUPP;
 
-	src_lock = nfs_get_lock_context(nfs_file_open_context(src_f));
+	src_lock = nfs_find_lock_context(nfs_file_open_context(src_f));
 	if (IS_ERR(src_lock))
 		return PTR_ERR(src_lock);
 
 	src_exception.inode = file_inode(src_f);
 	src_exception.state = src_lock->open_context->state;
 
-	dst_lock = nfs_get_lock_context(nfs_file_open_context(dst_f));
+	dst_lock = nfs_find_lock_context(nfs_file_open_context(dst_f));
 	if (IS_ERR(dst_lock)) {
 		err = PTR_ERR(dst_lock);
 		goto out_put_src_lock;

fs/nfs/pagelist.c +1 -1

index 8ce4f61..b3e5366 100644
@@ -329,7 +329,7 @@ nfs_create_request(struct nfs_open_context *ctx, struct page *page,
 		return ERR_PTR(-ENOMEM);
 
 	/* get lock context early so we can deal with alloc failures */
-	l_ctx = nfs_get_lock_context(ctx);
+	l_ctx = nfs_find_lock_context(ctx);
 	if (IS_ERR(l_ctx)) {
 		nfs_page_free(req);
 		return ERR_CAST(l_ctx);

include/linux/nfs_fs.h +2 -1

index 67300f8..e93e285 100644
@@ -365,7 +365,8 @@ extern struct nfs_open_context *alloc_nfs_open_context(struct dentry *dentry, fm
 extern void nfs_inode_attach_open_context(struct nfs_open_context *ctx);
 extern void nfs_file_set_open_context(struct file *filp, struct nfs_open_context *ctx);
 extern void nfs_file_clear_open_context(struct file *flip);
-extern struct nfs_lock_context *nfs_get_lock_context(struct nfs_open_context *ctx);
+extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
+extern struct nfs_lock_context *get_nfs_lock_context(struct nfs_lock_context *l_ctx);
 extern void nfs_put_lock_context(struct nfs_lock_context *l_ctx);
 extern u64 nfs_compat_user_ino64(u64 fileid);
 extern void nfs_fattr_init(struct nfs_fattr *fattr);

Signature · 1 line

1.7.1

Benjamin Coddington

author

2016-04-01 15:34 UTC #4 lore raw

PATCH 2/3 NFS: add OFD lock owners to nfs_lock_context

In reply to Benjamin Coddington

OFD locks use the file pointer for lock ownership. Change lock context lookup to take a file pointer which will create distinct lock contexts for operations on separate files. This allows us to retain the OFD lock owner so we can find a matching nfs4_lock_state later.

IO paths are modified to lookup or create the lock context earlier in the call path where we have a reference to a file pointer.

Signed-off-by Benjamin Coddington <[email protected]>

---
 fs/nfs/direct.c          |    8 ++++----
 fs/nfs/file.c            |    2 +-
 fs/nfs/fscache-index.c   |    4 ++--
 fs/nfs/fscache.h         |   12 ++++++------
 fs/nfs/inode.c           |   23 ++++++++++++++---------
 fs/nfs/nfs42proc.c       |    8 ++++----
 fs/nfs/nfs4proc.c        |    3 ++-
 fs/nfs/nfs4state.c       |    2 +-
 fs/nfs/pagelist.c        |   21 +++++++--------------
 fs/nfs/read.c            |   38 +++++++++++++++++++++++---------------
 fs/nfs/write.c           |   16 +++++++++-------
 include/linux/nfs_fs.h   |    5 +++--
 include/linux/nfs_page.h |    2 +-
 13 files changed, 77 insertions(+), 67 deletions(-)

fs/nfs/direct.c +4 -4

index 5f833fa..8f49e95 100644
@@ -499,7 +499,7 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
 			struct nfs_page *req;
 			unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
 			/* XXX do we need to do the eof zeroing found in async_filler? */
-			req = nfs_create_request(dreq->ctx, pagevec[i], NULL,
+			req = nfs_create_request(dreq->l_ctx, pagevec[i], NULL,
 						 pgbase, req_len);
 			if (IS_ERR(req)) {
 				result = PTR_ERR(req);

@@ -596,7 +596,7 @@ ssize_t nfs_file_direct_read(struct kiocb *iocb, struct iov_iter *iter,
 	dreq->bytes_left = count;
 	dreq->io_start = pos;
 	dreq->ctx = get_nfs_open_context(nfs_file_open_context(iocb->ki_filp));
-	l_ctx = nfs_find_lock_context(dreq->ctx);
+	l_ctx = nfs_find_lock_context(file);
 	if (IS_ERR(l_ctx)) {
 		result = PTR_ERR(l_ctx);
 		goto out_release;

@@ -915,7 +915,7 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
 			struct nfs_page *req;
 			unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
 
-			req = nfs_create_request(dreq->ctx, pagevec[i], NULL,
+			req = nfs_create_request(dreq->l_ctx, pagevec[i], NULL,
 						 pgbase, req_len);
 			if (IS_ERR(req)) {
 				result = PTR_ERR(req);

@@ -1029,7 +1029,7 @@ ssize_t nfs_file_direct_write(struct kiocb *iocb, struct iov_iter *iter)
 	dreq->bytes_left = iov_iter_count(iter);
 	dreq->io_start = pos;
 	dreq->ctx = get_nfs_open_context(nfs_file_open_context(iocb->ki_filp));
-	l_ctx = nfs_find_lock_context(dreq->ctx);
+	l_ctx = nfs_find_lock_context(file);
 	if (IS_ERR(l_ctx)) {
 		result = PTR_ERR(l_ctx);
 		goto out_release;

fs/nfs/file.c +1 -1

index f99c085..c1361c6 100644
@@ -754,7 +754,7 @@ do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 	 */
 	vfs_fsync(filp, 0);
 
-	l_ctx = nfs_find_lock_context(nfs_file_open_context(filp));
+	l_ctx = nfs_find_lock_context(filp);
 	if (!IS_ERR(l_ctx)) {
 		status = nfs_iocounter_wait(l_ctx);
 		nfs_put_lock_context(l_ctx);

fs/nfs/fscache-index.c +2 -2

index 777b055..c182d57 100644
@@ -300,7 +300,7 @@ static void nfs_fscache_inode_now_uncached(void *cookie_netfs_data)
  */
 static void nfs_fh_get_context(void *cookie_netfs_data, void *context)
 {
-	get_nfs_open_context(context);
+	get_nfs_lock_context(context);
 }
 
 /*

@@ -311,7 +311,7 @@ static void nfs_fh_get_context(void *cookie_netfs_data, void *context)
 static void nfs_fh_put_context(void *cookie_netfs_data, void *context)
 {
 	if (context)
-		put_nfs_open_context(context);
+		put_nfs_lock_context(context);
 }
 
 /*

fs/nfs/fscache.h +6 -6

index d7fe3e7..5e08751 100644
@@ -114,26 +114,26 @@ static inline void nfs_fscache_invalidate_page(struct page *page,
 /*
  * Retrieve a page from an inode data storage object.
  */
-static inline int nfs_readpage_from_fscache(struct nfs_open_context *ctx,
+static inline int nfs_readpage_from_fscache(struct nfs_lock_context *l_ctx,
 					    struct inode *inode,
 					    struct page *page)
 {
 	if (NFS_I(inode)->fscache)
-		return __nfs_readpage_from_fscache(ctx, inode, page);
+		return __nfs_readpage_from_fscache(l_ctx, inode, page);
 	return -ENOBUFS;
 }
 
 /*
  * Retrieve a set of pages from an inode data storage object.
  */
-static inline int nfs_readpages_from_fscache(struct nfs_open_context *ctx,
+static inline int nfs_readpages_from_fscache(struct nfs_lock_context *l_ctx,
 					     struct inode *inode,
 					     struct address_space *mapping,
 					     struct list_head *pages,
 					     unsigned *nr_pages)
 {
 	if (NFS_I(inode)->fscache)
-		return __nfs_readpages_from_fscache(ctx, inode, mapping, pages,
+		return __nfs_readpages_from_fscache(l_ctx, inode, mapping, pages,
 						    nr_pages);
 	return -ENOBUFS;
 }

@@ -199,13 +199,13 @@ static inline void nfs_fscache_invalidate_page(struct page *page,
 static inline void nfs_fscache_wait_on_page_write(struct nfs_inode *nfsi,
 						  struct page *page) {}
 
-static inline int nfs_readpage_from_fscache(struct nfs_open_context *ctx,
+static inline int nfs_readpage_from_fscache(struct nfs_lock_context *l_ctx,
 					    struct inode *inode,
 					    struct page *page)
 {
 	return -ENOBUFS;
 }
-static inline int nfs_readpages_from_fscache(struct nfs_open_context *ctx,
+static inline int nfs_readpages_from_fscache(struct nfs_lock_context *l_ctx,
 					     struct inode *inode,
 					     struct address_space *mapping,
 					     struct list_head *pages,

fs/nfs/inode.c +14 -9

index 5d484e5..a703a02 100644
@@ -700,22 +700,25 @@ out:
 }
 EXPORT_SYMBOL_GPL(nfs_getattr);
 
-static void nfs_init_lock_context(struct nfs_lock_context *l_ctx)
+static void nfs_init_lock_context(struct nfs_lock_context *l_ctx, struct file *file)
 {
 	atomic_set(&l_ctx->count, 1);
-	l_ctx->lockowner.l_owner = current->files;
+	l_ctx->lockowner.l_owner_posix = current->files;
+	l_ctx->lockowner.l_owner_ofd = file;
 	l_ctx->lockowner.l_pid = current->tgid;
 	INIT_LIST_HEAD(&l_ctx->list);
 	atomic_set(&l_ctx->io_count, 0);
 }
 
-static struct nfs_lock_context *__nfs_find_lock_context(struct nfs_open_context *ctx)
+static struct nfs_lock_context *__nfs_find_lock_context(struct nfs_open_context *ctx, struct file *file)
 {
 	struct nfs_lock_context *head = &ctx->lock_context;
 	struct nfs_lock_context *pos = head;
 
 	do {
-		if (pos->lockowner.l_owner != current->files)
+		if (pos->lockowner.l_owner_posix != current->files)
+			continue;
+		if (pos->lockowner.l_owner_ofd != file)
 			continue;
 		if (pos->lockowner.l_pid != current->tgid)
 			continue;

@@ -725,21 +728,22 @@ static struct nfs_lock_context *__nfs_find_lock_context(struct nfs_open_context
 	return NULL;
 }
 
-struct nfs_lock_context *nfs_find_lock_context(struct nfs_open_context *ctx)
+struct nfs_lock_context *nfs_find_lock_context(struct file *file)
 {
+	struct nfs_open_context *ctx = nfs_file_open_context(file);
 	struct nfs_lock_context *res, *new = NULL;
 	struct inode *inode = d_inode(ctx->dentry);
 
 	spin_lock(&inode->i_lock);
-	res = __nfs_find_lock_context(ctx);
+	res = __nfs_find_lock_context(ctx, file);
 	if (res == NULL) {
 		spin_unlock(&inode->i_lock);
 		new = kmalloc(sizeof(*new), GFP_KERNEL);
 		if (new == NULL)
 			return ERR_PTR(-ENOMEM);
-		nfs_init_lock_context(new);
+		nfs_init_lock_context(new, file);
 		spin_lock(&inode->i_lock);
-		res = __nfs_find_lock_context(ctx);
+		res = __nfs_find_lock_context(ctx, file);
 		if (res == NULL) {
 			list_add_tail(&new->list, &ctx->lock_context.list);
 			new->open_context = ctx;

@@ -826,7 +830,7 @@ struct nfs_open_context *alloc_nfs_open_context(struct dentry *dentry, fmode_t f
 	ctx->mode = f_mode;
 	ctx->flags = 0;
 	ctx->error = 0;
-	nfs_init_lock_context(&ctx->lock_context);
+	nfs_init_lock_context(&ctx->lock_context, NULL);
 	ctx->lock_context.open_context = ctx;
 	INIT_LIST_HEAD(&ctx->list);
 	ctx->mdsthreshold = NULL;

@@ -893,6 +897,7 @@ EXPORT_SYMBOL_GPL(nfs_inode_attach_open_context);
 void nfs_file_set_open_context(struct file *filp, struct nfs_open_context *ctx)
 {
 	filp->private_data = get_nfs_open_context(ctx);
+	ctx->lock_context.lockowner.l_owner_ofd = filp;
 	if (list_empty(&ctx->list))
 		nfs_inode_attach_open_context(ctx);
 }

fs/nfs/nfs42proc.c +4 -4

index f9b0f25..107a182 100644
@@ -61,7 +61,7 @@ static int nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
 	struct nfs_lock_context *lock;
 	int err;
 
-	lock = nfs_find_lock_context(nfs_file_open_context(filep));
+	lock = nfs_find_lock_context(filep);
 	if (IS_ERR(lock))
 		return PTR_ERR(lock);

@@ -171,7 +171,7 @@ loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
 	struct nfs_lock_context *lock;
 	loff_t err;
 
-	lock = nfs_find_lock_context(nfs_file_open_context(filep));
+	lock = nfs_find_lock_context(filep);
 	if (IS_ERR(lock))
 		return PTR_ERR(lock);

@@ -365,14 +365,14 @@ int nfs42_proc_clone(struct file *src_f, struct file *dst_f,
 	if (!nfs_server_capable(inode, NFS_CAP_CLONE))
 		return -EOPNOTSUPP;
 
-	src_lock = nfs_find_lock_context(nfs_file_open_context(src_f));
+	src_lock = nfs_find_lock_context(src_f);
 	if (IS_ERR(src_lock))
 		return PTR_ERR(src_lock);
 
 	src_exception.inode = file_inode(src_f);
 	src_exception.state = src_lock->open_context->state;
 
-	dst_lock = nfs_find_lock_context(nfs_file_open_context(dst_f));
+	dst_lock = nfs_find_lock_context(dst_f);
 	if (IS_ERR(dst_lock)) {
 		err = PTR_ERR(dst_lock);
 		goto out_put_src_lock;

fs/nfs/nfs4proc.c +2 -1

index 1488159..cb5b326 100644
@@ -2694,7 +2694,8 @@ static int _nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
 		/* Use that stateid */
 	} else if (truncate && state != NULL) {
 		struct nfs_lockowner lockowner = {
-			.l_owner = current->files,
+			.l_owner_posix = current->files,
+			.l_owner_ofd = sattr->ia_file,
 			.l_pid = current->tgid,
 		};
 		if (!nfs4_valid_open_stateid(state))

fs/nfs/nfs4state.c +1 -1

index d854693..b7f4509 100644
@@ -952,7 +952,7 @@ static int nfs4_copy_lock_stateid(nfs4_stateid *dst,
 	if (test_bit(LK_STATE_IN_USE, &state->flags) == 0)
 		goto out;
 
-	fl_owner = lockowner->l_owner;
+	fl_owner = lockowner->l_owner_posix;
 	spin_lock(&state->state_lock);
 	lsp = __nfs4_find_lock_state(state, fl_owner);
 	if (lsp && test_bit(NFS_LOCK_LOST, &lsp->ls_flags))

fs/nfs/pagelist.c +7 ~~-14~~

index b3e5366..2b28dff 100644
@@ -314,27 +314,20 @@ nfs_page_group_destroy(struct kref *kref)
  * User should ensure it is safe to sleep in this function.
  */
 struct nfs_page *
-nfs_create_request(struct nfs_open_context *ctx, struct page *page,
+nfs_create_request(struct nfs_lock_context *l_ctx, struct page *page,
 		   struct nfs_page *last, unsigned int offset,
 		   unsigned int count)
 {
 	struct nfs_page		*req;
-	struct nfs_lock_context *l_ctx;
 
-	if (test_bit(NFS_CONTEXT_BAD, &ctx->flags))
+	if (test_bit(NFS_CONTEXT_BAD, &l_ctx->open_context->flags))
 		return ERR_PTR(-EBADF);
 	/* try to allocate the request struct */
 	req = nfs_page_alloc();
 	if (req == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	/* get lock context early so we can deal with alloc failures */
-	l_ctx = nfs_find_lock_context(ctx);
-	if (IS_ERR(l_ctx)) {
-		nfs_page_free(req);
-		return ERR_CAST(l_ctx);
-	}
-	req->wb_lock_context = l_ctx;
+	req->wb_lock_context = get_nfs_lock_context(l_ctx);
 	atomic_inc(&l_ctx->io_count);
 
 	/* Initialize the request struct. Initially, we assume a

@@ -346,7 +339,7 @@ nfs_create_request(struct nfs_open_context *ctx, struct page *page,
 	req->wb_offset  = offset;
 	req->wb_pgbase	= offset;
 	req->wb_bytes   = count;
-	req->wb_context = get_nfs_open_context(ctx);
+	req->wb_context = get_nfs_open_context(l_ctx->open_context);
 	kref_init(&req->wb_kref);
 	nfs_page_group_init(req, last);
 	return req;

@@ -865,7 +858,7 @@ static void nfs_pageio_cleanup_mirroring(struct nfs_pageio_descriptor *pgio)
 static bool nfs_match_lock_context(const struct nfs_lock_context *l1,
 		const struct nfs_lock_context *l2)
 {
-	return l1->lockowner.l_owner == l2->lockowner.l_owner
+	return l1->lockowner.l_owner_posix == l2->lockowner.l_owner_posix
 		&& l1->lockowner.l_pid == l2->lockowner.l_pid;
 }

@@ -1024,7 +1017,7 @@ static int __nfs_pageio_add_request(struct nfs_pageio_descriptor *desc,
 		pgbase += subreq->wb_bytes;
 
 		if (bytes_left) {
-			subreq = nfs_create_request(req->wb_context,
+			subreq = nfs_create_request(req->wb_lock_context,
 					req->wb_page,
 					subreq, pgbase, bytes_left);
 			if (IS_ERR(subreq))

@@ -1115,7 +1108,7 @@ int nfs_pageio_add_request(struct nfs_pageio_descriptor *desc,
 			     lastreq = lastreq->wb_this_page)
 				;
 
-			dupreq = nfs_create_request(req->wb_context,
+			dupreq = nfs_create_request(req->wb_lock_context,
 					req->wb_page, lastreq, pgbase, bytes);
 
 			if (IS_ERR(dupreq)) {

fs/nfs/read.c +23 ~~-15~~

index eb31e23..704effd 100644
@@ -102,7 +102,7 @@ static void nfs_readpage_release(struct nfs_page *req)
 	nfs_release_request(req);
 }
 
-int nfs_readpage_async(struct nfs_open_context *ctx, struct inode *inode,
+int nfs_readpage_async(struct nfs_lock_context *l_ctx, struct inode *inode,
 		       struct page *page)
 {
 	struct nfs_page	*new;

@@ -113,7 +113,7 @@ int nfs_readpage_async(struct nfs_open_context *ctx, struct inode *inode,
 	len = nfs_page_length(page);
 	if (len == 0)
 		return nfs_return_empty_page(page);
-	new = nfs_create_request(ctx, page, NULL, 0, len);
+	new = nfs_create_request(l_ctx, page, NULL, 0, len);
 	if (IS_ERR(new)) {
 		unlock_page(page);
 		return PTR_ERR(new);

@@ -291,6 +291,7 @@ static void nfs_readpage_result(struct rpc_task *task,
 int nfs_readpage(struct file *file, struct page *page)
 {
 	struct nfs_open_context *ctx;
+	struct nfs_lock_context *l_ctx;
 	struct inode *inode = page_file_mapping(page)->host;
 	int		error;

@@ -321,19 +322,22 @@ int nfs_readpage(struct file *file, struct page *page)
 		ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
 		if (ctx == NULL)
 			goto out_unlock;
-	} else
-		ctx = get_nfs_open_context(nfs_file_open_context(file));
+		l_ctx = get_nfs_lock_context(&ctx->lock_context);
+		put_nfs_open_context(ctx);
+	} else {
+		l_ctx = nfs_find_lock_context(file);
+	}
 
 	if (!IS_SYNC(inode)) {
-		error = nfs_readpage_from_fscache(ctx, inode, page);
+		error = nfs_readpage_from_fscache(l_ctx, inode, page);
 		if (error == 0)
 			goto out;
 	}
 
-	error = nfs_readpage_async(ctx, inode, page);
+	error = nfs_readpage_async(l_ctx, inode, page);
 
 out:
-	put_nfs_open_context(ctx);
+	nfs_put_lock_context(l_ctx);
 	return error;
 out_unlock:
 	unlock_page(page);

@@ -342,7 +346,7 @@ out_unlock:
 
 struct nfs_readdesc {
 	struct nfs_pageio_descriptor *pgio;
-	struct nfs_open_context *ctx;
+	struct nfs_lock_context *l_ctx;
 };
 
 static int

@@ -357,7 +361,7 @@ readpage_async_filler(void *data, struct page *page)
 	if (len == 0)
 		return nfs_return_empty_page(page);
 
-	new = nfs_create_request(desc->ctx, page, NULL, 0, len);
+	new = nfs_create_request(desc->l_ctx, page, NULL, 0, len);
 	if (IS_ERR(new))
 		goto out_error;

@@ -382,6 +386,7 @@ int nfs_readpages(struct file *filp, struct address_space *mapping,
 {
 	struct nfs_pageio_descriptor pgio;
 	struct nfs_pgio_mirror *pgm;
+	struct nfs_open_context *ctx;
 	struct nfs_readdesc desc = {
 		.pgio = &pgio,
 	};

@@ -399,16 +404,19 @@ int nfs_readpages(struct file *filp, struct address_space *mapping,
 		goto out;
 
 	if (filp == NULL) {
-		desc.ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
-		if (desc.ctx == NULL)
+		ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
+		if (ctx == NULL)
 			return -EBADF;
-	} else
-		desc.ctx = get_nfs_open_context(nfs_file_open_context(filp));
+		desc.l_ctx = get_nfs_lock_context(&ctx->lock_context);
+		put_nfs_open_context(ctx);
+	} else {
+		desc.l_ctx = nfs_find_lock_context(filp);
+	}
 
 	/* attempt to read as many of the pages as possible from the cache
 	 * - this returns -ENOBUFS immediately if the cookie is negative
 	 */
-	ret = nfs_readpages_from_fscache(desc.ctx, inode, mapping,
+	ret = nfs_readpages_from_fscache(desc.l_ctx, inode, mapping,
 					 pages, &nr_pages);
 	if (ret == 0)
 		goto read_complete; /* all pages were read */

@@ -428,7 +436,7 @@ int nfs_readpages(struct file *filp, struct address_space *mapping,
 		 PAGE_CACHE_SHIFT;
 	nfs_add_stats(inode, NFSIOS_READPAGES, npages);
 read_complete:
-	put_nfs_open_context(desc.ctx);
+	nfs_put_lock_context(desc.l_ctx);
 out:
 	return ret;
 }

fs/nfs/write.c +9 -7

index 5754835..935244c 100644
@@ -1101,7 +1101,7 @@ out_err:
  * if we have to add a new request. Also assumes that the caller has
  * already called nfs_flush_incompatible() if necessary.
  */
-static struct nfs_page * nfs_setup_write_request(struct nfs_open_context* ctx,
+static struct nfs_page * nfs_setup_write_request(struct nfs_lock_context *l_ctx,
 		struct page *page, unsigned int offset, unsigned int bytes)
 {
 	struct inode *inode = page_file_mapping(page)->host;

@@ -1110,7 +1110,7 @@ static struct nfs_page * nfs_setup_write_request(struct nfs_open_context* ctx,
 	req = nfs_try_to_update_request(inode, page, offset, bytes);
 	if (req != NULL)
 		goto out;
-	req = nfs_create_request(ctx, page, NULL, offset, bytes);
+	req = nfs_create_request(l_ctx, page, NULL, offset, bytes);
 	if (IS_ERR(req))
 		goto out;
 	nfs_inode_add_request(inode, req);

@@ -1118,12 +1118,12 @@ out:
 	return req;
 }
 
-static int nfs_writepage_setup(struct nfs_open_context *ctx, struct page *page,
+static int nfs_writepage_setup(struct nfs_lock_context *l_ctx, struct page *page,
 		unsigned int offset, unsigned int count)
 {
 	struct nfs_page	*req;
 
-	req = nfs_setup_write_request(ctx, page, offset, count);
+	req = nfs_setup_write_request(l_ctx, page, offset, count);
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 	/* Update file length */

@@ -1161,7 +1161,7 @@ int nfs_flush_incompatible(struct file *file, struct page *page)
 		if (l_ctx && flctx &&
 		    !(list_empty_careful(&flctx->flc_posix) &&
 		      list_empty_careful(&flctx->flc_flock))) {
-			do_flush |= l_ctx->lockowner.l_owner != current->files
+			do_flush |= l_ctx->lockowner.l_owner_posix != current->files
 				|| l_ctx->lockowner.l_pid != current->tgid;
 		}
 		nfs_release_request(req);

@@ -1279,7 +1279,7 @@ static int nfs_can_extend_write(struct file *file, struct page *page, struct ino
 int nfs_updatepage(struct file *file, struct page *page,
 		unsigned int offset, unsigned int count)
 {
-	struct nfs_open_context *ctx = nfs_file_open_context(file);
+	struct nfs_lock_context *l_ctx = nfs_find_lock_context(file);
 	struct inode	*inode = page_file_mapping(page)->host;
 	int		status = 0;

@@ -1293,12 +1293,14 @@ int nfs_updatepage(struct file *file, struct page *page,
 		offset = 0;
 	}
 
-	status = nfs_writepage_setup(ctx, page, offset, count);
+	status = nfs_writepage_setup(l_ctx, page, offset, count);
 	if (status < 0)
 		nfs_set_pageerror(page);
 	else
 		__set_page_dirty_nobuffers(page);
 
+	nfs_put_lock_context(l_ctx);
+
 	dprintk("NFS:       nfs_updatepage returns %d (isize %lld)\n",
 			status, (long long)i_size_read(inode));
 	return status;

include/linux/nfs_fs.h +3 -2

index e93e285..cf43e23 100644
@@ -56,7 +56,8 @@ struct nfs_access_entry {
 };
 
 struct nfs_lockowner {
-	fl_owner_t l_owner;
+	fl_owner_t l_owner_posix;
+	fl_owner_t l_owner_ofd;
 	pid_t l_pid;
 };

@@ -542,7 +543,7 @@ nfs_have_writebacks(struct inode *inode)
 extern int  nfs_readpage(struct file *, struct page *);
 extern int  nfs_readpages(struct file *, struct address_space *,
 		struct list_head *, unsigned);
-extern int  nfs_readpage_async(struct nfs_open_context *, struct inode *,
+extern int  nfs_readpage_async(struct nfs_lock_context *, struct inode *,
 			       struct page *);
 
 /*

include/linux/nfs_page.h +1 -1

index f2f650f..3b0cbcd 100644
@@ -110,7 +110,7 @@ struct nfs_pageio_descriptor {
 
 #define NFS_WBACK_BUSY(req)	(test_bit(PG_BUSY,&(req)->wb_flags))
 
-extern	struct nfs_page *nfs_create_request(struct nfs_open_context *ctx,
+extern	struct nfs_page *nfs_create_request(struct nfs_lock_context *l_ctx,
 					    struct page *page,
 					    struct nfs_page *last,
 					    unsigned int offset,

Signature · 1 line

1.7.1

Benjamin Coddington

author

2016-04-01 15:34 UTC #7 lore raw

PATCH 3/3 NFSv4: use OFD lock owners in lock state lookup

In reply to Benjamin Coddington

Now that our lock context contains owners for OFD locks we can use those owners to reference an appropriate nfs4_lock_state.

Signed-off-by Benjamin Coddington <[email protected]>

---
 fs/nfs/nfs4state.c |   21 ++++++++++++---------
 fs/nfs/pagelist.c  |    1 +
 fs/nfs/write.c     |    1 +
 3 files changed, 14 insertions(+), 9 deletions(-)

fs/nfs/nfs4state.c +12 -9

index b7f4509..c602e8f 100644
@@ -800,14 +800,16 @@ void nfs4_close_sync(struct nfs4_state *state, fmode_t fmode)
  * that is compatible with current->files
  */
 static struct nfs4_lock_state *
-__nfs4_find_lock_state(struct nfs4_state *state, fl_owner_t fl_owner)
+__nfs4_find_lock_state(struct nfs4_state *state,
+		const struct nfs_lockowner *lockowner)
 {
 	struct nfs4_lock_state *pos;
 	list_for_each_entry(pos, &state->lock_states, ls_locks) {
-		if (pos->ls_owner != fl_owner)
-			continue;
-		atomic_inc(&pos->ls_count);
-		return pos;
+		if (pos->ls_owner == lockowner->l_owner_posix ||
+			pos->ls_owner == lockowner->l_owner_ofd) {
+			atomic_inc(&pos->ls_count);
+			return pos;
+		}
 	}
 	return NULL;
 }

@@ -854,10 +856,13 @@ void nfs4_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp
 static struct nfs4_lock_state *nfs4_get_lock_state(struct nfs4_state *state, fl_owner_t owner)
 {
 	struct nfs4_lock_state *lsp, *new = NULL;
+	const struct nfs_lockowner lockowner = {
+		.l_owner_posix = owner,
+	};
 	
 	for(;;) {
 		spin_lock(&state->state_lock);
-		lsp = __nfs4_find_lock_state(state, owner);
+		lsp = __nfs4_find_lock_state(state, &lockowner);
 		if (lsp != NULL)
 			break;
 		if (new != NULL) {

@@ -942,7 +947,6 @@ static int nfs4_copy_lock_stateid(nfs4_stateid *dst,
 		const struct nfs_lockowner *lockowner)
 {
 	struct nfs4_lock_state *lsp;
-	fl_owner_t fl_owner;
 	int ret = -ENOENT;

@@ -952,9 +956,8 @@ static int nfs4_copy_lock_stateid(nfs4_stateid *dst,
 	if (test_bit(LK_STATE_IN_USE, &state->flags) == 0)
 		goto out;
 
-	fl_owner = lockowner->l_owner_posix;
 	spin_lock(&state->state_lock);
-	lsp = __nfs4_find_lock_state(state, fl_owner);
+	lsp = __nfs4_find_lock_state(state, lockowner);
 	if (lsp && test_bit(NFS_LOCK_LOST, &lsp->ls_flags))
 		ret = -EIO;
 	else if (lsp != NULL && test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags) != 0) {

fs/nfs/pagelist.c +1 -0

index 2b28dff..3579638 100644
@@ -859,6 +859,7 @@ static bool nfs_match_lock_context(const struct nfs_lock_context *l1,
 		const struct nfs_lock_context *l2)
 {
 	return l1->lockowner.l_owner_posix == l2->lockowner.l_owner_posix
+		&& l1->lockowner.l_owner_ofd == l2->lockowner.l_owner_ofd
 		&& l1->lockowner.l_pid == l2->lockowner.l_pid;
 }

fs/nfs/write.c +1 -0

index 935244c..2d01d6c 100644
@@ -1162,6 +1162,7 @@ int nfs_flush_incompatible(struct file *file, struct page *page)
 		    !(list_empty_careful(&flctx->flc_posix) &&
 		      list_empty_careful(&flctx->flc_flock))) {
 			do_flush |= l_ctx->lockowner.l_owner_posix != current->files
+				|| l_ctx->lockowner.l_owner_ofd != file
 				|| l_ctx->lockowner.l_pid != current->tgid;
 		}
 		nfs_release_request(req);

Signature · 1 line

1.7.1

Trond Myklebust

2016-04-01 15:47 UTC #8 lore raw

In reply to Benjamin Coddington

Quoted 5 lines · Benjamin Coddington

The client sends IO only under the open stateid when using OFD (and flock) locking instead of the appropriate lock stateid because the nfs_lock_context only tracks POSIX lockowners, which is the reference to the process' file table.

Quoted 8 lines

This is a problem for two reasons. The first is that rfc7530, section-9.1.4.5 states that IO sent by an entity corresponding to the lock-owner which holds a byte-range lock should be sent under the lock stateid for that lock. Otherwise, a server enforcing mandatory byte-range locking might reject that operation. Secondly, not tracking OFD lock owners means that accounting for IO sent under those owners is broken. That creates a problem for some work to guarantee an unlock will be sent after operations scheduled under a lock complete.

OK. Can we just kill this in the bud? No support for OFD locks in NFS:
this is nuts....

Benjamin Coddington

author

2016-04-01 15:48 UTC #9 lore raw

Quoted 16 lines · Trond Myklebust

Quoted 5 lines · Benjamin Coddington
The client sends IO only under the open stateid when using OFD (and flock) locking instead of the appropriate lock stateid because the nfs_lock_context only tracks POSIX lockowners, which is the reference to the process' file table.
Quoted 8 lines
This is a problem for two reasons. The first is that rfc7530, section-9.1.4.5 states that IO sent by an entity corresponding to the lock-owner which holds a byte-range lock should be sent under the lock stateid for that lock. Otherwise, a server enforcing mandatory byte-range locking might reject that operation. Secondly, not tracking OFD lock owners means that accounting for IO sent under those owners is broken. That creates a problem for some work to guarantee an unlock will be sent after operations scheduled under a lock complete.

Quoted 2 lines

OK. Can we just kill this in the bud? No support for OFD locks in NFS:
this is nuts....

Will you explain why it is nuts? That would be helpful for me.

Jeff Layton

2016-04-01 16:09 UTC #14 lore raw

In reply to Trond Myklebust

Quoted 16 lines · On Fri, 1 Apr 2016 11:47:10 -0400 Trond Myklebust…

Quoted 5 lines · Benjamin Coddington
The client sends IO only under the open stateid when using OFD (and flock) locking instead of the appropriate lock stateid because the nfs_lock_context only tracks POSIX lockowners, which is the reference to the process' file table.
Quoted 8 lines
This is a problem for two reasons. The first is that rfc7530, section-9.1.4.5 states that IO sent by an entity corresponding to the lock-owner which holds a byte-range lock should be sent under the lock stateid for that lock. Otherwise, a server enforcing mandatory byte-range locking might reject that operation. Secondly, not tracking OFD lock owners means that accounting for IO sent under those owners is broken. That creates a problem for some work to guarantee an unlock will be sent after operations scheduled under a lock complete.

Quoted 2 lines

OK. Can we just kill this in the bud? No support for OFD locks in NFS:
this is nuts....

Why wouldn't there be? OFD locks seem quite sane (more so than traditional POSIX locks anyway), and the opaque v4 lockowner model should work with OFD locks just fine...

Signature · 1 line

Jeff Layton <[email protected]>

Trond Myklebust

2016-04-01 16:09 UTC #10 lore raw

In reply to Benjamin Coddington

Quoted 18 lines · Benjamin Coddington

Quoted 16 lines · Trond Myklebust
Quoted 13 lines · Benjamin Coddington
The client sends IO only under the open stateid when using OFD (and flock) locking instead of the appropriate lock stateid because the nfs_lock_context only tracks POSIX lockowners, which is the reference to the process' file table.
This is a problem for two reasons. The first is that rfc7530, section-9.1.4.5 states that IO sent by an entity corresponding to the lock-owner which holds a byte-range lock should be sent under the lock stateid for that lock. Otherwise, a server enforcing mandatory byte-range locking might reject that operation. Secondly, not tracking OFD lock owners means that accounting for IO sent under those owners is broken. That creates a problem for some work to guarantee an unlock will be sent after operations scheduled under a lock complete.

Quoted 4 lines

Quoted 2 lines
OK. Can we just kill this in the bud? No support for OFD locks in NFS:
this is nuts....
Will you explain why it is nuts? That would be helpful for me.

The point of the OFD crap was that they should work exactly like POSIX locks except for the unlock-on-close, the latter being managed by the VFS layer. If we have to make loads of changes to NFS in order to change the tracking of lock owners, then the design itself is broken, and needs to be fixed.

kbuild test robot

build bot

2016-04-01 16:17 UTC #5 lore raw

Re: [PATCH 2/3] NFS: add OFD lock owners to nfs_lock_context

In reply to Benjamin Coddington

reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree…

Hi Benjamin,

[auto build test ERROR on v4.6-rc1]
[also build test ERROR on next-20160401]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Benjamin-Coddington/Include-OFD-lock-owners-when-looking-up-state/20160401-233801
config: sparc64-allmodconfig (attached as .config)

reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64

All error/warnings (new ones prefixed by >>):

In file included from fs/nfs/nfs4file.c:14:0:
fs/nfs/fscache.h: In function 'nfs_readpage_from_fscache':

>> fs/nfs/fscache.h:122:10: warning: passing argument 1 of '__nfs_readpage_from_fscache' from incompatible pointer type
      return __nfs_readpage_from_fscache(l_ctx, inode, page);
             ^
   fs/nfs/fscache.h:86:12: note: expected 'struct nfs_open_context *' but argument is of type 'struct nfs_lock_context *'
    extern int __nfs_readpage_from_fscache(struct nfs_open_context *,
               ^
   fs/nfs/fscache.h: In function 'nfs_readpages_from_fscache':
>> fs/nfs/fscache.h:136:10: warning: passing argument 1 of '__nfs_readpages_from_fscache' from incompatible pointer type

return __nfs_readpages_from_fscache(l_ctx, inode, mapping, pages,
^
fs/nfs/fscache.h:88:12: note: expected 'struct nfs_open_context *' but argument is of type 'struct nfs_lock_context *'
extern int __nfs_readpages_from_fscache(struct nfs_open_context *,
^
--
In file included from fs/nfs/fscache-index.c:21:0:
fs/nfs/fscache.h: In function 'nfs_readpage_from_fscache':

>> fs/nfs/fscache.h:122:10: warning: passing argument 1 of '__nfs_readpage_from_fscache' from incompatible pointer type
      return __nfs_readpage_from_fscache(l_ctx, inode, page);
             ^
   fs/nfs/fscache.h:86:12: note: expected 'struct nfs_open_context *' but argument is of type 'struct nfs_lock_context *'
    extern int __nfs_readpage_from_fscache(struct nfs_open_context *,
               ^
   fs/nfs/fscache.h: In function 'nfs_readpages_from_fscache':
>> fs/nfs/fscache.h:136:10: warning: passing argument 1 of '__nfs_readpages_from_fscache' from incompatible pointer type
      return __nfs_readpages_from_fscache(l_ctx, inode, mapping, pages,
             ^
   fs/nfs/fscache.h:88:12: note: expected 'struct nfs_open_context *' but argument is of type 'struct nfs_lock_context *'
    extern int __nfs_readpages_from_fscache(struct nfs_open_context *,
               ^
   fs/nfs/fscache-index.c: In function 'nfs_fh_put_context':
>> fs/nfs/fscache-index.c:314:3: error: implicit declaration of function 'put_nfs_lock_context' [-Werror=implicit-function-declaration]
      put_nfs_lock_context(context);
      ^

cc1: some warnings being treated as errors

vim +/put_nfs_lock_context +314 fs/nfs/fscache-index.c

308 * - This function can be absent if the completion function doesn't require a
309 * context.
310 */
311 static void nfs_fh_put_context(void *cookie_netfs_data, void *context)
312 {
313 if (context)

 > 314			put_nfs_lock_context(context);

315 }
316
317 /*

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Jeff Layton

2016-04-01 16:19 UTC #11 lore raw

In reply to Trond Myklebust

Quoted 23 lines · On Fri, 1 Apr 2016 12:09:31 -0400 Trond Myklebust…

Quoted 21 lines · Benjamin Coddington
Quoted 18 lines · Trond Myklebust
Quoted 5 lines · Benjamin Coddington
The client sends IO only under the open stateid when using OFD (and flock) locking instead of the appropriate lock stateid because the nfs_lock_context only tracks POSIX lockowners, which is the reference to the process' file table.
Quoted 8 lines
This is a problem for two reasons. The first is that rfc7530, section-9.1.4.5 states that IO sent by an entity corresponding to the lock-owner which holds a byte-range lock should be sent under the lock stateid for that lock. Otherwise, a server enforcing mandatory byte-range locking might reject that operation. Secondly, not tracking OFD lock owners means that accounting for IO sent under those owners is broken. That creates a problem for some work to guarantee an unlock will be sent after operations scheduled under a lock complete.
OK. Can we just kill this in the bud? No support for OFD locks in NFS:
this is nuts....

Quoted 7 lines

Quoted 1 line
Will you explain why it is nuts? That would be helpful for me.
The point of the OFD crap was that they should work exactly like POSIX locks except for the unlock-on-close, the latter being managed by the VFS layer. If we have to make loads of changes to NFS in order to change the tracking of lock owners, then the design itself is broken, and needs to be fixed.

No, there were other reasons for them as well.

For instance, traditional POSIX locks are useless for serializing betwee threads within the same process because the lock owner is always the same regardless of which thread you're using. With OFD locks, this is possible if each thread has its own file description.

Signature · 1 line

Jeff Layton <[email protected]>

Benjamin Coddington

author

2016-04-01 16:35 UTC #13 lore raw

In reply to Trond Myklebust

Quoted 23 lines · Trond Myklebust

Quoted 21 lines · Benjamin Coddington
Quoted 18 lines · Trond Myklebust
Quoted 5 lines · Benjamin Coddington
The client sends IO only under the open stateid when using OFD (and flock) locking instead of the appropriate lock stateid because the nfs_lock_context only tracks POSIX lockowners, which is the reference to the process' file table.
Quoted 8 lines
This is a problem for two reasons. The first is that rfc7530, section-9.1.4.5 states that IO sent by an entity corresponding to the lock-owner which holds a byte-range lock should be sent under the lock stateid for that lock. Otherwise, a server enforcing mandatory byte-range locking might reject that operation. Secondly, not tracking OFD lock owners means that accounting for IO sent under those owners is broken. That creates a problem for some work to guarantee an unlock will be sent after operations scheduled under a lock complete.
OK. Can we just kill this in the bud? No support for OFD locks in NFS:
this is nuts....

Quoted 7 lines

Quoted 1 line
Will you explain why it is nuts? That would be helpful for me.
The point of the OFD crap was that they should work exactly like POSIX locks except for the unlock-on-close, the latter being managed by the VFS layer. If we have to make loads of changes to NFS in order to change the tracking of lock owners, then the design itself is broken, and needs to be fixed.

It seems your objection is more about trying to combine the two different models of a lock's ownership or boundaries from the VFS side, where in NFS there's been traditionally only one model used.

The reason that OFD locks require changes to work on NFS is because the owners (the context of a lock) are different, and NFS right now assumes that the boundaries of lock ownership are always going to be a shared file table.

Assuming that the boundaries of a lock will only ever be entities with shared file tables might be difficult to support long term. What happens when applications start using OFD locks (or other future locks that might allow user-defined locking contexts) and then later we discover applications won't work properly on NFS?

How can we fix this once for NFS? NFS could be fixed if it didn't have to keep track of the lock context, if the lock context were opaque. Should there be a way to look up a reference to a locking context without having to keep track of files or file tables or anything else?

Ben

kbuild test robot

build bot

2016-04-01 16:45 UTC #2 lore raw

Re: [PATCH 1/3] NFS: add get_nfs_lock_context, find_nfs_lock_context

In reply to Benjamin Coddington

reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree…

Hi Benjamin,

[auto build test ERROR on v4.6-rc1]
[also build test ERROR on next-20160401]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Benjamin-Coddington/Include-OFD-lock-owners-when-looking-up-state/20160401-233801
config: mips-jz4740 (attached as .config)

Note: the linux-review/Benjamin-Coddington/Include-OFD-lock-owners-when-looking-up-state/20160401-233801 HEAD e6973cc5e04cb6af43dbba362969dbf6ddcf7740 builds fine.
It only hurts bisectibility.

All errors (new ones prefixed by >>):

fs/nfs/file.c: In function 'do_unlk':

>> fs/nfs/file.c:759:32: error: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type [-Werror=incompatible-pointer-types]
     l_ctx = nfs_find_lock_context(nfs_file_open_context(filp));
                                   ^
   In file included from fs/nfs/file.c:25:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
   cc1: some warnings being treated as errors
--
   fs/nfs/direct.c: In function 'nfs_file_direct_read':
>> fs/nfs/direct.c:599:32: error: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type [-Werror=incompatible-pointer-types]
     l_ctx = nfs_find_lock_context(dreq->ctx);
                                   ^
   In file included from fs/nfs/direct.c:51:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
   fs/nfs/direct.c: In function 'nfs_file_direct_write':
   fs/nfs/direct.c:1032:32: error: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type [-Werror=incompatible-pointer-types]
     l_ctx = nfs_find_lock_context(dreq->ctx);
                                   ^
   In file included from fs/nfs/direct.c:51:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
   cc1: some warnings being treated as errors
--
   fs/nfs/pagelist.c: In function 'nfs_create_request':
>> fs/nfs/pagelist.c:332:32: error: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type [-Werror=incompatible-pointer-types]
     l_ctx = nfs_find_lock_context(ctx);
                                   ^
   In file included from fs/nfs/pagelist.c:20:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^

cc1: some warnings being treated as errors

vim +/nfs_find_lock_context +759 fs/nfs/file.c

753 /*
754 * Flush all pending writes before doing anything
755 * with locks..
756 */
757 vfs_fsync(filp, 0);
758

 > 759		l_ctx = nfs_find_lock_context(nfs_file_open_context(filp));

760 if (!IS_ERR(l_ctx)) {
761 status = nfs_iocounter_wait(l_ctx);
762 nfs_put_lock_context(l_ctx);

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

kbuild test robot

build bot

2016-04-01 16:55 UTC #6 lore raw

Re: [PATCH 2/3] NFS: add OFD lock owners to nfs_lock_context

In reply to Benjamin Coddington

reproduce:

Hi Benjamin,

[auto build test ERROR on v4.6-rc1]
[also build test ERROR on next-20160401]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Benjamin-Coddington/Include-OFD-lock-owners-when-looking-up-state/20160401-233801
config: i386-randconfig-s0-201613 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386

All errors (new ones prefixed by >>):

In file included from fs/nfs/inode.c:49:0:
fs/nfs/fscache.h: In function 'nfs_readpage_from_fscache':

>> fs/nfs/fscache.h:122:38: error: passing argument 1 of '__nfs_readpage_from_fscache' from incompatible pointer type [-Werror=incompatible-pointer-types]
      return __nfs_readpage_from_fscache(l_ctx, inode, page);
                                         ^
   fs/nfs/fscache.h:86:12: note: expected 'struct nfs_open_context *' but argument is of type 'struct nfs_lock_context *'
    extern int __nfs_readpage_from_fscache(struct nfs_open_context *,
               ^
   fs/nfs/fscache.h: In function 'nfs_readpages_from_fscache':
>> fs/nfs/fscache.h:136:39: error: passing argument 1 of '__nfs_readpages_from_fscache' from incompatible pointer type [-Werror=incompatible-pointer-types]

return __nfs_readpages_from_fscache(l_ctx, inode, mapping, pages,
^
fs/nfs/fscache.h:88:12: note: expected 'struct nfs_open_context *' but argument is of type 'struct nfs_lock_context *'
extern int __nfs_readpages_from_fscache(struct nfs_open_context *,
^
cc1: some warnings being treated as errors
--
In file included from fs/nfs/fscache-index.c:21:0:
fs/nfs/fscache.h: In function 'nfs_readpage_from_fscache':

>> fs/nfs/fscache.h:122:38: error: passing argument 1 of '__nfs_readpage_from_fscache' from incompatible pointer type [-Werror=incompatible-pointer-types]
      return __nfs_readpage_from_fscache(l_ctx, inode, page);
                                         ^
   fs/nfs/fscache.h:86:12: note: expected 'struct nfs_open_context *' but argument is of type 'struct nfs_lock_context *'
    extern int __nfs_readpage_from_fscache(struct nfs_open_context *,
               ^
   fs/nfs/fscache.h: In function 'nfs_readpages_from_fscache':
>> fs/nfs/fscache.h:136:39: error: passing argument 1 of '__nfs_readpages_from_fscache' from incompatible pointer type [-Werror=incompatible-pointer-types]
      return __nfs_readpages_from_fscache(l_ctx, inode, mapping, pages,
                                          ^
   fs/nfs/fscache.h:88:12: note: expected 'struct nfs_open_context *' but argument is of type 'struct nfs_lock_context *'
    extern int __nfs_readpages_from_fscache(struct nfs_open_context *,
               ^
   fs/nfs/fscache-index.c: In function 'nfs_fh_put_context':
   fs/nfs/fscache-index.c:314:3: error: implicit declaration of function 'put_nfs_lock_context' [-Werror=implicit-function-declaration]
      put_nfs_lock_context(context);
      ^

cc1: some warnings being treated as errors

vim +/__nfs_readpage_from_fscache +122 fs/nfs/fscache.h

116 */
117 static inline int nfs_readpage_from_fscache(struct nfs_lock_context *l_ctx,
118 struct inode *inode,
119 struct page *page)
120 {
121 if (NFS_I(inode)->fscache)

 > 122			return __nfs_readpage_from_fscache(l_ctx, inode, page);
   123		return -ENOBUFS;
   124	}
   125	
   126	/*
   127	 * Retrieve a set of pages from an inode data storage object.
   128	 */
   129	static inline int nfs_readpages_from_fscache(struct nfs_lock_context *l_ctx,
   130						     struct inode *inode,
   131						     struct address_space *mapping,
   132						     struct list_head *pages,
   133						     unsigned *nr_pages)
   134	{
   135		if (NFS_I(inode)->fscache)
 > 136			return __nfs_readpages_from_fscache(l_ctx, inode, mapping, pages,

137 nr_pages);
138 return -ENOBUFS;
139 }

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

kbuild test robot

build bot

2016-04-01 17:05 UTC #3 lore raw

Re: [PATCH 1/3] NFS: add get_nfs_lock_context, find_nfs_lock_context

In reply to Benjamin Coddington

reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree…

Hi Benjamin,

[auto build test ERROR on v4.6-rc1]
[also build test ERROR on next-20160401]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Benjamin-Coddington/Include-OFD-lock-owners-when-looking-up-state/20160401-233801
config: sparc64-allyesconfig (attached as .config)

Note: the linux-review/Benjamin-Coddington/Include-OFD-lock-owners-when-looking-up-state/20160401-233801 HEAD e6973cc5e04cb6af43dbba362969dbf6ddcf7740 builds fine.
It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

fs/nfs/file.c: In function 'do_unlk':

>> fs/nfs/file.c:759:10: warning: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type
     l_ctx = nfs_find_lock_context(nfs_file_open_context(filp));
             ^
   In file included from fs/nfs/file.c:25:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
--
>> fs/nfs/inode.c:728:26: error: conflicting types for 'nfs_find_lock_context'
    struct nfs_lock_context *nfs_find_lock_context(struct nfs_open_context *ctx)
                             ^
   In file included from fs/nfs/inode.c:29:0:
   include/linux/nfs_fs.h:368:33: note: previous declaration of 'nfs_find_lock_context' was here
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
   In file included from include/linux/linkage.h:6:0,
                    from include/linux/kernel.h:6,
                    from include/linux/list.h:8,
                    from include/linux/module.h:9,
                    from fs/nfs/inode.c:16:
   fs/nfs/inode.c:754:19: error: conflicting types for 'nfs_find_lock_context'
    EXPORT_SYMBOL_GPL(nfs_find_lock_context);
                      ^
   include/linux/export.h:57:21: note: in definition of macro '__EXPORT_SYMBOL'
     extern typeof(sym) sym;     \
                        ^
>> fs/nfs/inode.c:754:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
    EXPORT_SYMBOL_GPL(nfs_find_lock_context);
    ^
   In file included from fs/nfs/inode.c:29:0:
   include/linux/nfs_fs.h:368:33: note: previous declaration of 'nfs_find_lock_context' was here
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
--
   fs/nfs/direct.c: In function 'nfs_file_direct_read':
>> fs/nfs/direct.c:599:10: warning: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type
     l_ctx = nfs_find_lock_context(dreq->ctx);
             ^
   In file included from fs/nfs/direct.c:51:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
   fs/nfs/direct.c: In function 'nfs_file_direct_write':
   fs/nfs/direct.c:1032:10: warning: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type
     l_ctx = nfs_find_lock_context(dreq->ctx);
             ^
   In file included from fs/nfs/direct.c:51:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
--
   fs/nfs/pagelist.c: In function 'nfs_create_request':
>> fs/nfs/pagelist.c:332:10: warning: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type
     l_ctx = nfs_find_lock_context(ctx);
             ^
   In file included from fs/nfs/pagelist.c:20:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
--
   fs/nfs/nfs42proc.c: In function 'nfs42_proc_fallocate':
>> fs/nfs/nfs42proc.c:64:9: warning: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type
     lock = nfs_find_lock_context(nfs_file_open_context(filep));
            ^
   In file included from fs/nfs/nfs42proc.c:10:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
   fs/nfs/nfs42proc.c: In function 'nfs42_proc_llseek':
   fs/nfs/nfs42proc.c:174:9: warning: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type
     lock = nfs_find_lock_context(nfs_file_open_context(filep));
            ^
   In file included from fs/nfs/nfs42proc.c:10:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
   fs/nfs/nfs42proc.c: In function 'nfs42_proc_clone':
   fs/nfs/nfs42proc.c:368:13: warning: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type
     src_lock = nfs_find_lock_context(nfs_file_open_context(src_f));
                ^
   In file included from fs/nfs/nfs42proc.c:10:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^
   fs/nfs/nfs42proc.c:375:13: warning: passing argument 1 of 'nfs_find_lock_context' from incompatible pointer type
     dst_lock = nfs_find_lock_context(nfs_file_open_context(dst_f));
                ^
   In file included from fs/nfs/nfs42proc.c:10:0:
   include/linux/nfs_fs.h:368:33: note: expected 'struct file *' but argument is of type 'struct nfs_open_context *'
    extern struct nfs_lock_context *nfs_find_lock_context(struct file *file);
                                    ^

vim +/nfs_find_lock_context +728 fs/nfs/inode.c

722 atomic_inc(&pos->count);
723 return pos;
724 } while ((pos = list_entry(pos->list.next, typeof(*pos), list)) != head);
725 return NULL;
726 }
727

 > 728	struct nfs_lock_context *nfs_find_lock_context(struct nfs_open_context *ctx)
   729	{
   730		struct nfs_lock_context *res, *new = NULL;
   731		struct inode *inode = d_inode(ctx->dentry);
   732	
   733		spin_lock(&inode->i_lock);
   734		res = __nfs_find_lock_context(ctx);
   735		if (res == NULL) {
   736			spin_unlock(&inode->i_lock);
   737			new = kmalloc(sizeof(*new), GFP_KERNEL);
   738			if (new == NULL)
   739				return ERR_PTR(-ENOMEM);
   740			nfs_init_lock_context(new);
   741			spin_lock(&inode->i_lock);
   742			res = __nfs_find_lock_context(ctx);
   743			if (res == NULL) {
   744				list_add_tail(&new->list, &ctx->lock_context.list);
   745				new->open_context = ctx;
   746				res = new;
   747				new = NULL;
   748			}
   749		}
   750		spin_unlock(&inode->i_lock);
   751		kfree(new);
   752		return res;
   753	}
 > 754	EXPORT_SYMBOL_GPL(nfs_find_lock_context);

755
756 struct nfs_lock_context *get_nfs_lock_context(struct nfs_lock_context *l_ctx)
757 {

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Benjamin Coddington

author

2016-04-01 17:17 UTC #15 lore raw

In reply to Benjamin Coddington

kbuild robot noticed I made a mistake doing a quick rebase. I'll send a V2 for completeness even if it is disliked. I hope we can keep discussing it..

Ben

Quoted 58 lines · Benjamin Coddington

The client sends IO only under the open stateid when using OFD (and flock) locking instead of the appropriate lock stateid because the nfs_lock_context only tracks POSIX lockowners, which is the reference to the process' file table.
This is a problem for two reasons. The first is that rfc7530, section-9.1.4.5 states that IO sent by an entity corresponding to the lock-owner which holds a byte-range lock should be sent under the lock stateid for that lock. Otherwise, a server enforcing mandatory byte-range locking might reject that operation. Secondly, not tracking OFD lock owners means that accounting for IO sent under those owners is broken. That creates a problem for some work to guarantee an unlock will be sent after operations scheduled under a lock complete.
To fix this, this set starts tracking the OFD lock owner within the nfs_lock_context. Unique nfs_lock_contexts are created on distinct combinations of current->files, struct file, and pid. In order to do this, some re-arrangement of when to create or lookup the nfs_lock_context was needed since the struct file pointer must be included during that creation or lookup.
It is possible for a process to hold both an OFD and a POSIX lock on a file as long as they do not conflict. There would be two stateids that could be selected for IO operations on that file. In that case only the first stateid found to match the current lock context will ever be used. The result of this is that mixed OFD and POSIX locks within the same process on a server enforcing mandatory locking should be avoided. The fix for this probably would require a byte-range lookup of stateid when scheduling an IO operation, and the ability to split IO that crossed lock ranges with different owners.
Comments and critique is welcomed.
Benjamin Coddington (3):
NFS: add get_nfs_lock_context, find_nfs_lock_context
NFS: add OFD lock owners to nfs_lock_context
NFSv4: use OFD lock owners in lock state lookup
 fs/nfs/direct.c          |    8 ++++----
 fs/nfs/file.c            |    2 +-
 fs/nfs/fscache-index.c   |    4 ++--
 fs/nfs/fscache.h         |   12 ++++++------
 fs/nfs/inode.c           |   32 ++++++++++++++++++++++----------
 fs/nfs/nfs42proc.c       |    8 ++++----
 fs/nfs/nfs4proc.c        |    3 ++-
 fs/nfs/nfs4state.c       |   21 ++++++++++++---------
 fs/nfs/pagelist.c        |   22 ++++++++--------------
 fs/nfs/read.c            |   38 +++++++++++++++++++++++---------------
 fs/nfs/write.c           |   17 ++++++++++-------
 include/linux/nfs_fs.h   |    8 +++++---
 include/linux/nfs_page.h |    2 +-
 13 files changed, 100 insertions(+), 77 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Frank Filz

2016-04-01 20:24 UTC #12 lore raw

In reply to Jeff Layton

Quoted 19 lines

Quoted 11 lines
Quoted 9 lines · Benjamin Coddington
Quoted 7 lines · Trond Myklebust
Quoted 5 lines · Benjamin Coddington
The client sends IO only under the open stateid when using OFD (and flock) locking instead of the appropriate lock stateid because the nfs_lock_context only tracks POSIX lockowners, which is the reference to the process' file table.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
This is a problem for two reasons. The first is that rfc7530, section-9.1.4.5 states that IO sent by an entity corresponding to the lock-owner which holds a byte-range lock should be sent under the lock stateid for that lock. Otherwise, a server enforcing mandatory byte-range locking might reject that operation. Secondly, not tracking OFD lock owners means that accounting for IO sent under those owners is broken. That creates a problem for some work to guarantee an unlock will be sent after operations

Quoted 3 lines

scheduled under a lock complete.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
OK. Can we just kill this in the bud? No support for OFD locks in

NFS:

Quoted 11 lines

Quoted 9 lines
Quoted 3 lines
Quoted 1 line
this is nuts....
Will you explain why it is nuts? That would be helpful for me.
The point of the OFD crap was that they should work exactly like POSIX locks except for the unlock-on-close, the latter being managed by the VFS layer. If we have to make loads of changes to NFS in order to change the tracking of lock owners, then the design itself is broken, and needs to be fixed.

Quoted 5 lines

No, there were other reasons for them as well.
For instance, traditional POSIX locks are useless for serializing betwee threads within the same process because the lock owner is always the same regardless of which thread you're using. With OFD locks, this is possible

Quoted 1 line

each thread has its own file description.

Which is a feature humongously useful for Ganesha... Not that Ganesha will ever use OFD locks over NFS...

I have used OFD locks over NFS to make sure that Ganesha handles one open-owner to many lock-owners correctly.

Of course it may be a real question if anyone other than Ganesha (and maybe Samba) will ever use OFD locks since they aren't a POSIX standard...

Frank

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus