[PATCH 0/3] NFSD: Add ALLOCATE and DEALLOCATE support

[PATCH 0/3] NFSD: Add ALLOCATE and DEALLOCATE support

[Anna.Schumaker]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

These patches add server support for the ALLOCATE and DEALLOCATE operations
part of NFS v4.2.

The first patch in this series renames a VFS-level function for consistency
with how other VFS functions are named.  Who is the correct person to carry
this patch?

The remaining patches add the new operations.

Anna

Anna Schumaker (3):
  VFS: Rename do_fallocate() to vfs_fallocate()
  nfsd: Add ALLOCATE support
  nfsd: Add DEALLOCATE support

 drivers/staging/android/ashmem.c |  2 +-
 fs/ioctl.c                       |  2 +-
 fs/nfsd/nfs4proc.c               | 58 ++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c                | 38 ++++++++++++++++++++++++--
 fs/nfsd/vfs.c                    | 13 +++++++++
 fs/nfsd/vfs.h                    |  1 +
 fs/nfsd/xdr4.h                   | 16 +++++++++++
 fs/open.c                        |  5 ++--
 include/linux/fs.h               |  2 +-
 mm/madvise.c                     |  2 +-
 10 files changed, 131 insertions(+), 8 deletions(-)

-- 
2.1.0

[PATCH 1/3] VFS: Rename do_fallocate() to vfs_fallocate()

[Anna.Schumaker]

From: Anna Schumaker <Anna.Schumaker@netapp.com>

This function needs to be exported so it can be used by the NFSD module
when responding to ALLOCATE and DEALLOCATE operations.  Christoph Hellwig
suggested renaming the function to match how other vfs functions are named.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 drivers/staging/android/ashmem.c | 2 +-
 fs/ioctl.c                       | 2 +-
 fs/open.c                        | 5 +++--
 include/linux/fs.h               | 2 +-
 mm/madvise.c                     | 2 +-
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 713a972..9c799987 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -446,7 +446,7 @@ ashmem_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 		loff_t start = range->pgstart * PAGE_SIZE;
 		loff_t end = (range->pgend + 1) * PAGE_SIZE;
 
-		do_fallocate(range->asma->file,
+		vfs_fallocate(range->asma->file,
 				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 				start, end - start);
 		range->purged = ASHMEM_WAS_PURGED;
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 8ac3fad..0bd61421 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -443,7 +443,7 @@ int ioctl_preallocate(struct file *filp, void __user *argp)
 		return -EINVAL;
 	}
 
-	return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start, sr.l_len);
+	return vfs_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start, sr.l_len);
 }
 
 static int file_ioctl(struct file *filp, unsigned int cmd,
diff --git a/fs/open.c b/fs/open.c
index d6fd3ac..c94449b 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -222,7 +222,7 @@ SYSCALL_DEFINE2(ftruncate64, unsigned int, fd, loff_t, length)
 #endif /* BITS_PER_LONG == 32 */
 
 
-int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
+int vfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 {
 	struct inode *inode = file_inode(file);
 	long ret;
@@ -298,6 +298,7 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 	sb_end_write(inode->i_sb);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(vfs_fallocate);
 
 SYSCALL_DEFINE4(fallocate, int, fd, int, mode, loff_t, offset, loff_t, len)
 {
@@ -305,7 +306,7 @@ SYSCALL_DEFINE4(fallocate, int, fd, int, mode, loff_t, offset, loff_t, len)
 	int error = -EBADF;
 
 	if (f.file) {
-		error = do_fallocate(f.file, mode, offset, len);
+		error = vfs_fallocate(f.file, mode, offset, len);
 		fdput(f);
 	}
 	return error;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9418772..476f555 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2049,7 +2049,7 @@ struct filename {
 extern long vfs_truncate(struct path *, loff_t);
 extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs,
 		       struct file *filp);
-extern int do_fallocate(struct file *file, int mode, loff_t offset,
+extern int vfs_fallocate(struct file *file, int mode, loff_t offset,
 			loff_t len);
 extern long do_sys_open(int dfd, const char __user *filename, int flags,
 			umode_t mode);
diff --git a/mm/madvise.c b/mm/madvise.c
index 0938b30..a271adc 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -326,7 +326,7 @@ static long madvise_remove(struct vm_area_struct *vma,
 	 */
 	get_file(f);
 	up_read(&current->mm->mmap_sem);
-	error = do_fallocate(f,
+	error = vfs_fallocate(f,
 				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 				offset, end - start);
 	fput(f);
-- 
2.1.0

[PATCH 2/3] nfsd: Add ALLOCATE support

[Anna.Schumaker]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

The ALLOCATE operation is used to preallocate space in a file.  I use
vfs_fallocate() to do the actual preallocation.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfsd/nfs4proc.c | 29 +++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c  | 19 ++++++++++++++++++-
 fs/nfsd/vfs.c      |  9 +++++++++
 fs/nfsd/vfs.h      |  1 +
 fs/nfsd/xdr4.h     |  8 ++++++++
 5 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 9fd2b78..f9d8fac 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1014,6 +1014,28 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 }
 
 static __be32
+nfsd4_allocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+		struct nfsd4_allocate *allocate)
+{
+	__be32 status = nfserr_notsupp;
+	struct file *file;
+
+	status = nfs4_preprocess_stateid_op(SVC_NET(rqstp), cstate,
+					    &allocate->alloc_stateid,
+					    WR_STATE, &file);
+	if (status != nfs_ok) {
+		dprintk("NFSD: nfsd4_allocate: couldn't process stateid!\n");
+		return status;
+	}
+
+	status = nfsd4_vfs_fallocate(&cstate->current_fh, file,
+				     allocate->alloc_offset,
+				     allocate->alloc_length);
+	fput(file);
+	return status;
+}
+
+static __be32
 nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		struct nfsd4_seek *seek)
 {
@@ -1920,6 +1942,13 @@ static struct nfsd4_operation nfsd4_ops[] = {
 	},
 
 	/* NFSv4.2 operations */
+	[OP_ALLOCATE] = {
+		.op_func = (nfsd4op_func)nfsd4_allocate,
+		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+		.op_name = "OP_ALLOCATE",
+		.op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
+		.op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
+	},
 	[OP_SEEK] = {
 		.op_func = (nfsd4op_func)nfsd4_seek,
 		.op_name = "OP_SEEK",
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 57b3973..a55aa3b 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1521,6 +1521,23 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
 }
 
 static __be32
+nfsd4_decode_allocate(struct nfsd4_compoundargs *argp,
+		      struct nfsd4_allocate *allocate)
+{
+	DECODE_HEAD;
+
+	status = nfsd4_decode_stateid(argp, &allocate->alloc_stateid);
+	if (status)
+		return status;
+
+	READ_BUF(16);
+	p = xdr_decode_hyper(p, &allocate->alloc_offset);
+	xdr_decode_hyper(p, &allocate->alloc_length);
+
+	DECODE_TAIL;
+}
+
+static __be32
 nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
 {
 	DECODE_HEAD;
@@ -1611,7 +1628,7 @@ static nfsd4_dec nfsd4_dec_ops[] = {
 	[OP_RECLAIM_COMPLETE]	= (nfsd4_dec)nfsd4_decode_reclaim_complete,
 
 	/* new operations for NFSv4.2 */
-	[OP_ALLOCATE]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_ALLOCATE]		= (nfsd4_dec)nfsd4_decode_allocate,
 	[OP_COPY]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_COPY_NOTIFY]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_DEALLOCATE]		= (nfsd4_dec)nfsd4_decode_notsupp,
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f501a9b..4b1dba0 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -16,6 +16,7 @@
 #include <linux/fs.h>
 #include <linux/file.h>
 #include <linux/splice.h>
+#include <linux/falloc.h>
 #include <linux/fcntl.h>
 #include <linux/namei.h>
 #include <linux/delay.h>
@@ -523,6 +524,14 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
 }
 #endif
 
+__be32 nfsd4_vfs_fallocate(struct svc_fh *fhp, struct file *file,
+			   loff_t offset, loff_t len)
+{
+	int error = vfs_fallocate(file, 0, offset, len);
+	if (error < 0)
+		return nfserrno(error);
+	return nfserrno(commit_metadata(fhp));
+}
 #endif /* defined(CONFIG_NFSD_V4) */
 
 #ifdef CONFIG_NFSD_V3
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index c2ff3f1..9d88c88 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -54,6 +54,7 @@ int nfsd_mountpoint(struct dentry *, struct svc_export *);
 #ifdef CONFIG_NFSD_V4
 __be32          nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
 		    struct xdr_netobj *);
+__be32		nfsd4_vfs_fallocate(struct svc_fh *, struct file *, loff_t, loff_t);
 #endif /* CONFIG_NFSD_V4 */
 __be32		nfsd_create(struct svc_rqst *, struct svc_fh *,
 				char *name, int len, struct iattr *attrs,
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 5720e94..4a01fa8 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -428,6 +428,13 @@ struct nfsd4_reclaim_complete {
 	u32 rca_one_fs;
 };
 
+struct nfsd4_allocate {
+	/* request */
+	stateid_t	alloc_stateid;
+	loff_t		alloc_offset;
+	u64		alloc_length;
+};
+
 struct nfsd4_seek {
 	/* request */
 	stateid_t	seek_stateid;
@@ -486,6 +493,7 @@ struct nfsd4_op {
 		struct nfsd4_free_stateid	free_stateid;
 
 		/* NFSv4.2 */
+		struct nfsd4_allocate		allocate;
 		struct nfsd4_seek		seek;
 	} u;
 	struct nfs4_replay *			replay;
-- 
2.1.0

[PATCH 3/3] nfsd: Add DEALLOCATE support

[Anna.Schumaker]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

The DEALLOCATE operation is used to punch holes in a file.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++++-
 fs/nfsd/nfs4xdr.c  | 19 ++++++++++++++++++-
 fs/nfsd/vfs.c      |  8 ++++++--
 fs/nfsd/vfs.h      |  2 +-
 fs/nfsd/xdr4.h     |  8 ++++++++
 5 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index f9d8fac..ac18be6 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1028,7 +1028,7 @@ nfsd4_allocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		return status;
 	}
 
-	status = nfsd4_vfs_fallocate(&cstate->current_fh, file,
+	status = nfsd4_vfs_fallocate(&cstate->current_fh, file, false,
 				     allocate->alloc_offset,
 				     allocate->alloc_length);
 	fput(file);
@@ -1036,6 +1036,28 @@ nfsd4_allocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 }
 
 static __be32
+nfsd4_deallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+		struct nfsd4_deallocate *deallocate)
+{
+	__be32 status = nfserr_notsupp;
+	struct file *file;
+
+	status = nfs4_preprocess_stateid_op(SVC_NET(rqstp), cstate,
+					    &deallocate->dealloc_stateid,
+					    WR_STATE, &file);
+	if (status != nfs_ok) {
+		dprintk("NFSD: nfsd4_deallocate: couldn't process stateid!\n");
+		return status;
+	}
+
+	status = nfsd4_vfs_fallocate(&cstate->current_fh, file, true,
+				     deallocate->dealloc_offset,
+				     deallocate->dealloc_length);
+	fput(file);
+	return status;
+}
+
+static __be32
 nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		struct nfsd4_seek *seek)
 {
@@ -1949,6 +1971,13 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
 		.op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
 	},
+	[OP_DEALLOCATE] = {
+		.op_func = (nfsd4op_func)nfsd4_deallocate,
+		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+		.op_name = "OP_DEALLOCATE",
+		.op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
+		.op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
+	},
 	[OP_SEEK] = {
 		.op_func = (nfsd4op_func)nfsd4_seek,
 		.op_name = "OP_SEEK",
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index a55aa3b..e1059ce 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1538,6 +1538,23 @@ nfsd4_decode_allocate(struct nfsd4_compoundargs *argp,
 }
 
 static __be32
+nfsd4_decode_deallocate(struct nfsd4_compoundargs *argp,
+		      struct nfsd4_deallocate *deallocate)
+{
+	DECODE_HEAD;
+
+	status = nfsd4_decode_stateid(argp, &deallocate->dealloc_stateid);
+	if (status)
+		return status;
+
+	READ_BUF(16);
+	p = xdr_decode_hyper(p, &deallocate->dealloc_offset);
+	xdr_decode_hyper(p, &deallocate->dealloc_length);
+
+	DECODE_TAIL;
+}
+
+static __be32
 nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
 {
 	DECODE_HEAD;
@@ -1631,7 +1648,7 @@ static nfsd4_dec nfsd4_dec_ops[] = {
 	[OP_ALLOCATE]		= (nfsd4_dec)nfsd4_decode_allocate,
 	[OP_COPY]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_COPY_NOTIFY]	= (nfsd4_dec)nfsd4_decode_notsupp,
-	[OP_DEALLOCATE]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_DEALLOCATE]		= (nfsd4_dec)nfsd4_decode_deallocate,
 	[OP_IO_ADVISE]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_LAYOUTERROR]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_LAYOUTSTATS]	= (nfsd4_dec)nfsd4_decode_notsupp,
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 4b1dba0..2bf5263 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -525,9 +525,13 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
 #endif
 
 __be32 nfsd4_vfs_fallocate(struct svc_fh *fhp, struct file *file,
-			   loff_t offset, loff_t len)
+			   bool punch, loff_t offset, loff_t len)
 {
-	int error = vfs_fallocate(file, 0, offset, len);
+	int error, mode = 0;
+	if (punch)
+		mode = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE;
+
+	error = vfs_fallocate(file, mode, offset, len);
 	if (error < 0)
 		return nfserrno(error);
 	return nfserrno(commit_metadata(fhp));
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 9d88c88..9301e8e 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -54,7 +54,7 @@ int nfsd_mountpoint(struct dentry *, struct svc_export *);
 #ifdef CONFIG_NFSD_V4
 __be32          nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
 		    struct xdr_netobj *);
-__be32		nfsd4_vfs_fallocate(struct svc_fh *, struct file *, loff_t, loff_t);
+__be32		nfsd4_vfs_fallocate(struct svc_fh *, struct file *, bool, loff_t, loff_t);
 #endif /* CONFIG_NFSD_V4 */
 __be32		nfsd_create(struct svc_rqst *, struct svc_fh *,
 				char *name, int len, struct iattr *attrs,
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 4a01fa8..2866921 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -435,6 +435,13 @@ struct nfsd4_allocate {
 	u64		alloc_length;
 };
 
+struct nfsd4_deallocate {
+	/* request */
+	stateid_t	dealloc_stateid;
+	loff_t		dealloc_offset;
+	u64		dealloc_length;
+};
+
 struct nfsd4_seek {
 	/* request */
 	stateid_t	seek_stateid;
@@ -494,6 +501,7 @@ struct nfsd4_op {
 
 		/* NFSv4.2 */
 		struct nfsd4_allocate		allocate;
+		struct nfsd4_deallocate		deallocate;
 		struct nfsd4_seek		seek;
 	} u;
 	struct nfs4_replay *			replay;
-- 
2.1.0

Re: [PATCH 1/3] VFS: Rename do_fallocate() to vfs_fallocate()

[Christoph Hellwig]

On Tue, Sep 09, 2014 at 10:49:40AM -0400, Anna.Schumaker@netapp.com wrote:
> From: Anna Schumaker <Anna.Schumaker@netapp.com>
> 
> This function needs to be exported so it can be used by the NFSD module
> when responding to ALLOCATE and DEALLOCATE operations.  Christoph Hellwig
> suggested renaming the function to match how other vfs functions are named.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/3] nfsd: Add ALLOCATE support

[Christoph Hellwig]

On Tue, Sep 09, 2014 at 10:49:41AM -0400, Anna.Schumaker@netapp.com wrote:
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> The ALLOCATE operation is used to preallocate space in a file.  I use
> vfs_fallocate() to do the actual preallocation.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>

Looks largely good, but can you do an audit how the error codes match
the spec?  E.g. We return -ENODEV in do_fallocate if it's called on
a special file, but that doesn't translate to any valid error codes
for ALLOCATE in the NFSv4.2.

I guess it should be NFSERR_NOTSUPP, and I'll work with Tom to actually
define this case in the spec.

Re: [PATCH 3/3] nfsd: Add DEALLOCATE support

[Christoph Hellwig]

> -	status = nfsd4_vfs_fallocate(&cstate->current_fh, file,
> +	status = nfsd4_vfs_fallocate(&cstate->current_fh, file, false,
>  				     allocate->alloc_offset,
>  				     allocate->alloc_length);

Maye just pass the normal Linux fallocate flags to nfsd4_vfs_fallocate
for future extensibility?

Otherwise looks as good as the previous one.

Re: [PATCH 0/3] NFSD: Add ALLOCATE and DEALLOCATE support

[Christoph Hellwig]

Sorry to hijack this thread, but talking about 4.2 server features, did
anyone into looking into implementing the change_attr_type attribute,
which should be fairly low hanging fruit?

Also someone with interest in btrfs might want to look into implementing
the space_freed attribute with a call into the filesystem.

Re: [PATCH 0/3] NFSD: Add ALLOCATE and DEALLOCATE support

[J. Bruce Fields]

On Tue, Sep 09, 2014 at 11:13:02AM -0700, Christoph Hellwig wrote:
> Sorry to hijack this thread, but talking about 4.2 server features, did
> anyone into looking into implementing the change_attr_type attribute,
> which should be fairly low hanging fruit?

I haven't seen anything.  Agreed that it should be easy (on the server
side--the client's use might be more complicated?).

--b.

> 
> Also someone with interest in btrfs might want to look into implementing
> the space_freed attribute with a call into the filesystem.
> 

Re: [PATCH 2/3] nfsd: Add ALLOCATE support

[Anna Schumaker]

On 09/09/2014 11:55 AM, Christoph Hellwig wrote:
> On Tue, Sep 09, 2014 at 10:49:41AM -0400, Anna.Schumaker@netapp.com wrote:
>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>
>> The ALLOCATE operation is used to preallocate space in a file.  I use
>> vfs_fallocate() to do the actual preallocation.
>>
>> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> Looks largely good, but can you do an audit how the error codes match
> the spec?  E.g. We return -ENODEV in do_fallocate if it's called on
> a special file, but that doesn't translate to any valid error codes
> for ALLOCATE in the NFSv4.2.

Good idea, I'll look into it!

Anna
> 
> I guess it should be NFSERR_NOTSUPP, and I'll work with Tom to actually
> define this case in the spec.
> 

Re: [PATCH 1/3] VFS: Rename do_fallocate() to vfs_fallocate()

[J. Bruce Fields]

On Tue, Sep 09, 2014 at 08:50:19AM -0700, Christoph Hellwig wrote:
> On Tue, Sep 09, 2014 at 10:49:40AM -0400, Anna.Schumaker@netapp.com wrote:
> > From: Anna Schumaker <Anna.Schumaker@netapp.com>
> > 
> > This function needs to be exported so it can be used by the NFSD module
> > when responding to ALLOCATE and DEALLOCATE operations.  Christoph Hellwig
> > suggested renaming the function to match how other vfs functions are named.
> > 
> > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

I'd like to be able to take this into through the nfsd tree if the nfsd
patches are going to depend on it.

Anna, on next posting maybe you could make sure to cc: the reelevant
people (I'm not sure who...) so I can get the right ACKs for that?

--b.

Re: [PATCH 2/3] nfsd: Add ALLOCATE support

[J. Bruce Fields]

On Tue, Sep 09, 2014 at 10:49:41AM -0400, Anna.Schumaker@netapp.com wrote:
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> The ALLOCATE operation is used to preallocate space in a file.  I use
> vfs_fallocate() to do the actual preallocation.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> ---
>  fs/nfsd/nfs4proc.c | 29 +++++++++++++++++++++++++++++
>  fs/nfsd/nfs4xdr.c  | 19 ++++++++++++++++++-
>  fs/nfsd/vfs.c      |  9 +++++++++
>  fs/nfsd/vfs.h      |  1 +
>  fs/nfsd/xdr4.h     |  8 ++++++++
>  5 files changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 9fd2b78..f9d8fac 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1014,6 +1014,28 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  }
>  
>  static __be32
> +nfsd4_allocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> +		struct nfsd4_allocate *allocate)
> +{
> +	__be32 status = nfserr_notsupp;
> +	struct file *file;
> +
> +	status = nfs4_preprocess_stateid_op(SVC_NET(rqstp), cstate,
> +					    &allocate->alloc_stateid,
> +					    WR_STATE, &file);

This checks that the stateid's openmode is write but not that the caller
has permission to write.  I think you want an nfsd_permission() check in
nfsd4_vfs_allocate.

(Incidentally, the spec doesn't say anything about checking either, I
wonder if it should?)

> +	if (status != nfs_ok) {
> +		dprintk("NFSD: nfsd4_allocate: couldn't process stateid!\n");
> +		return status;
> +	}
> +
> +	status = nfsd4_vfs_fallocate(&cstate->current_fh, file,
> +				     allocate->alloc_offset,
> +				     allocate->alloc_length);
> +	fput(file);
> +	return status;
> +}
> +
> +static __be32
>  nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		struct nfsd4_seek *seek)
>  {
> @@ -1920,6 +1942,13 @@ static struct nfsd4_operation nfsd4_ops[] = {
>  	},
>  
>  	/* NFSv4.2 operations */
> +	[OP_ALLOCATE] = {
> +		.op_func = (nfsd4op_func)nfsd4_allocate,
> +		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
> +		.op_name = "OP_ALLOCATE",
> +		.op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
> +		.op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
> +	},
>  	[OP_SEEK] = {
>  		.op_func = (nfsd4op_func)nfsd4_seek,
>  		.op_name = "OP_SEEK",
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 57b3973..a55aa3b 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1521,6 +1521,23 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
>  }
>  
>  static __be32
> +nfsd4_decode_allocate(struct nfsd4_compoundargs *argp,
> +		      struct nfsd4_allocate *allocate)
> +{
> +	DECODE_HEAD;
> +
> +	status = nfsd4_decode_stateid(argp, &allocate->alloc_stateid);
> +	if (status)
> +		return status;
> +
> +	READ_BUF(16);
> +	p = xdr_decode_hyper(p, &allocate->alloc_offset);
> +	xdr_decode_hyper(p, &allocate->alloc_length);
> +
> +	DECODE_TAIL;
> +}
> +
> +static __be32
>  nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
>  {
>  	DECODE_HEAD;
> @@ -1611,7 +1628,7 @@ static nfsd4_dec nfsd4_dec_ops[] = {
>  	[OP_RECLAIM_COMPLETE]	= (nfsd4_dec)nfsd4_decode_reclaim_complete,
>  
>  	/* new operations for NFSv4.2 */
> -	[OP_ALLOCATE]		= (nfsd4_dec)nfsd4_decode_notsupp,
> +	[OP_ALLOCATE]		= (nfsd4_dec)nfsd4_decode_allocate,
>  	[OP_COPY]		= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_COPY_NOTIFY]	= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_DEALLOCATE]		= (nfsd4_dec)nfsd4_decode_notsupp,
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index f501a9b..4b1dba0 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -16,6 +16,7 @@
>  #include <linux/fs.h>
>  #include <linux/file.h>
>  #include <linux/splice.h>
> +#include <linux/falloc.h>
>  #include <linux/fcntl.h>
>  #include <linux/namei.h>
>  #include <linux/delay.h>
> @@ -523,6 +524,14 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  }
>  #endif
>  
> +__be32 nfsd4_vfs_fallocate(struct svc_fh *fhp, struct file *file,
> +			   loff_t offset, loff_t len)
> +{
> +	int error = vfs_fallocate(file, 0, offset, len);
> +	if (error < 0)
> +		return nfserrno(error);
> +	return nfserrno(commit_metadata(fhp));
> +}
>  #endif /* defined(CONFIG_NFSD_V4) */
>  
>  #ifdef CONFIG_NFSD_V3
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index c2ff3f1..9d88c88 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -54,6 +54,7 @@ int nfsd_mountpoint(struct dentry *, struct svc_export *);
>  #ifdef CONFIG_NFSD_V4
>  __be32          nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
>  		    struct xdr_netobj *);
> +__be32		nfsd4_vfs_fallocate(struct svc_fh *, struct file *, loff_t, loff_t);
>  #endif /* CONFIG_NFSD_V4 */
>  __be32		nfsd_create(struct svc_rqst *, struct svc_fh *,
>  				char *name, int len, struct iattr *attrs,
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 5720e94..4a01fa8 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -428,6 +428,13 @@ struct nfsd4_reclaim_complete {
>  	u32 rca_one_fs;
>  };
>  
> +struct nfsd4_allocate {
> +	/* request */
> +	stateid_t	alloc_stateid;
> +	loff_t		alloc_offset;
> +	u64		alloc_length;
> +};
> +
>  struct nfsd4_seek {
>  	/* request */
>  	stateid_t	seek_stateid;
> @@ -486,6 +493,7 @@ struct nfsd4_op {
>  		struct nfsd4_free_stateid	free_stateid;
>  
>  		/* NFSv4.2 */
> +		struct nfsd4_allocate		allocate;
>  		struct nfsd4_seek		seek;
>  	} u;
>  	struct nfs4_replay *			replay;
> -- 
> 2.1.0
> 

Re: [PATCH 3/3] nfsd: Add DEALLOCATE support

[J. Bruce Fields]

On Tue, Sep 09, 2014 at 10:49:42AM -0400, Anna.Schumaker@netapp.com wrote:
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> The DEALLOCATE operation is used to punch holes in a file.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> ---
>  fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++++-
>  fs/nfsd/nfs4xdr.c  | 19 ++++++++++++++++++-
>  fs/nfsd/vfs.c      |  8 ++++++--
>  fs/nfsd/vfs.h      |  2 +-
>  fs/nfsd/xdr4.h     |  8 ++++++++
>  5 files changed, 63 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index f9d8fac..ac18be6 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1028,7 +1028,7 @@ nfsd4_allocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		return status;
>  	}
>  
> -	status = nfsd4_vfs_fallocate(&cstate->current_fh, file,
> +	status = nfsd4_vfs_fallocate(&cstate->current_fh, file, false,
>  				     allocate->alloc_offset,
>  				     allocate->alloc_length);
>  	fput(file);
> @@ -1036,6 +1036,28 @@ nfsd4_allocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  }
>  
>  static __be32
> +nfsd4_deallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> +		struct nfsd4_deallocate *deallocate)
> +{
> +	__be32 status = nfserr_notsupp;
> +	struct file *file;
> +
> +	status = nfs4_preprocess_stateid_op(SVC_NET(rqstp), cstate,
> +					    &deallocate->dealloc_stateid,
> +					    WR_STATE, &file);
> +	if (status != nfs_ok) {
> +		dprintk("NFSD: nfsd4_deallocate: couldn't process stateid!\n");
> +		return status;
> +	}
> +
> +	status = nfsd4_vfs_fallocate(&cstate->current_fh, file, true,
> +				     deallocate->dealloc_offset,
> +				     deallocate->dealloc_length);

Same comment as in allocate case.

--b.

> +	fput(file);
> +	return status;
> +}
> +
> +static __be32
>  nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		struct nfsd4_seek *seek)
>  {
> @@ -1949,6 +1971,13 @@ static struct nfsd4_operation nfsd4_ops[] = {
>  		.op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
>  		.op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
>  	},
> +	[OP_DEALLOCATE] = {
> +		.op_func = (nfsd4op_func)nfsd4_deallocate,
> +		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
> +		.op_name = "OP_DEALLOCATE",
> +		.op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
> +		.op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
> +	},
>  	[OP_SEEK] = {
>  		.op_func = (nfsd4op_func)nfsd4_seek,
>  		.op_name = "OP_SEEK",
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index a55aa3b..e1059ce 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1538,6 +1538,23 @@ nfsd4_decode_allocate(struct nfsd4_compoundargs *argp,
>  }
>  
>  static __be32
> +nfsd4_decode_deallocate(struct nfsd4_compoundargs *argp,
> +		      struct nfsd4_deallocate *deallocate)
> +{
> +	DECODE_HEAD;
> +
> +	status = nfsd4_decode_stateid(argp, &deallocate->dealloc_stateid);
> +	if (status)
> +		return status;
> +
> +	READ_BUF(16);
> +	p = xdr_decode_hyper(p, &deallocate->dealloc_offset);
> +	xdr_decode_hyper(p, &deallocate->dealloc_length);
> +
> +	DECODE_TAIL;
> +}
> +
> +static __be32
>  nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
>  {
>  	DECODE_HEAD;
> @@ -1631,7 +1648,7 @@ static nfsd4_dec nfsd4_dec_ops[] = {
>  	[OP_ALLOCATE]		= (nfsd4_dec)nfsd4_decode_allocate,
>  	[OP_COPY]		= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_COPY_NOTIFY]	= (nfsd4_dec)nfsd4_decode_notsupp,
> -	[OP_DEALLOCATE]		= (nfsd4_dec)nfsd4_decode_notsupp,
> +	[OP_DEALLOCATE]		= (nfsd4_dec)nfsd4_decode_deallocate,
>  	[OP_IO_ADVISE]		= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_LAYOUTERROR]	= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_LAYOUTSTATS]	= (nfsd4_dec)nfsd4_decode_notsupp,
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 4b1dba0..2bf5263 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -525,9 +525,13 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  #endif
>  
>  __be32 nfsd4_vfs_fallocate(struct svc_fh *fhp, struct file *file,
> -			   loff_t offset, loff_t len)
> +			   bool punch, loff_t offset, loff_t len)
>  {
> -	int error = vfs_fallocate(file, 0, offset, len);
> +	int error, mode = 0;
> +	if (punch)
> +		mode = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE;
> +
> +	error = vfs_fallocate(file, mode, offset, len);
>  	if (error < 0)
>  		return nfserrno(error);
>  	return nfserrno(commit_metadata(fhp));
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index 9d88c88..9301e8e 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -54,7 +54,7 @@ int nfsd_mountpoint(struct dentry *, struct svc_export *);
>  #ifdef CONFIG_NFSD_V4
>  __be32          nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
>  		    struct xdr_netobj *);
> -__be32		nfsd4_vfs_fallocate(struct svc_fh *, struct file *, loff_t, loff_t);
> +__be32		nfsd4_vfs_fallocate(struct svc_fh *, struct file *, bool, loff_t, loff_t);
>  #endif /* CONFIG_NFSD_V4 */
>  __be32		nfsd_create(struct svc_rqst *, struct svc_fh *,
>  				char *name, int len, struct iattr *attrs,
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 4a01fa8..2866921 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -435,6 +435,13 @@ struct nfsd4_allocate {
>  	u64		alloc_length;
>  };
>  
> +struct nfsd4_deallocate {
> +	/* request */
> +	stateid_t	dealloc_stateid;
> +	loff_t		dealloc_offset;
> +	u64		dealloc_length;
> +};
> +
>  struct nfsd4_seek {
>  	/* request */
>  	stateid_t	seek_stateid;
> @@ -494,6 +501,7 @@ struct nfsd4_op {
>  
>  		/* NFSv4.2 */
>  		struct nfsd4_allocate		allocate;
> +		struct nfsd4_deallocate		deallocate;
>  		struct nfsd4_seek		seek;
>  	} u;
>  	struct nfs4_replay *			replay;
> -- 
> 2.1.0
> 

Re: [PATCH 2/3] nfsd: Add ALLOCATE support

[Christoph Hellwig]

On Wed, Sep 10, 2014 at 04:18:12PM -0400, J. Bruce Fields wrote:
> This checks that the stateid's openmode is write but not that the caller
> has permission to write.  I think you want an nfsd_permission() check in
> nfsd4_vfs_allocate.
> 
> (Incidentally, the spec doesn't say anything about checking either, I
> wonder if it should?)

Yes, it probably should.

Re: [PATCH 1/3] VFS: Rename do_fallocate() to vfs_fallocate()

[Christoph Hellwig]

On Wed, Sep 10, 2014 at 03:43:42PM -0400, J. Bruce Fields wrote:
> On Tue, Sep 09, 2014 at 08:50:19AM -0700, Christoph Hellwig wrote:
> > On Tue, Sep 09, 2014 at 10:49:40AM -0400, Anna.Schumaker@netapp.com wrote:
> > > From: Anna Schumaker <Anna.Schumaker@netapp.com>
> > > 
> > > This function needs to be exported so it can be used by the NFSD module
> > > when responding to ALLOCATE and DEALLOCATE operations.  Christoph Hellwig
> > > suggested renaming the function to match how other vfs functions are named.
> > > 
> > > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > 
> > Looks good,
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> I'd like to be able to take this into through the nfsd tree if the nfsd
> patches are going to depend on it.
> 
> Anna, on next posting maybe you could make sure to cc: the reelevant
> people (I'm not sure who...) so I can get the right ACKs for that?

Cc Al and fsdevel, please.

Re: [PATCH 3/3] nfsd: Add DEALLOCATE support

[Christoph Hellwig]

On Wed, Sep 10, 2014 at 04:19:05PM -0400, J. Bruce Fields wrote:
> Same comment as in allocate case.

The nfsd_permission check probably should go into nfsd_vfs_fallocate
to avoid duplication.

Re: [PATCH 2/3] nfsd: Add ALLOCATE support

[Christoph Hellwig]

On Wed, Sep 10, 2014 at 03:11:50PM -0700, Christoph Hellwig wrote:
> On Wed, Sep 10, 2014 at 04:18:12PM -0400, J. Bruce Fields wrote:
> > This checks that the stateid's openmode is write but not that the caller
> > has permission to write.  I think you want an nfsd_permission() check in
> > nfsd4_vfs_allocate.
> > 
> > (Incidentally, the spec doesn't say anything about checking either, I
> > wonder if it should?)
> 
> Yes, it probably should.

Turns out the spec (at least 4.1) doesn't mention anything about
requiring a writeable stateid or permissions for WRITE either as
far as I can tell..