Re: [PATCH v7 11/11] xfstests: fsx: Add fallocate insert range operation

[Brian Foster]

On Fri, Jan 02, 2015 at 06:43:07PM +0900, Namjae Jeon wrote:
> This commit adds fallocate FALLOC_FL_INSERT_RANGE support for fsx.
> 
> Signed-off-by: Namjae Jeon <namjae.jeon@xxxxxxxxxxx>
> Signed-off-by: Ashish Sangwan <a.sangwan@xxxxxxxxxxx>
> ---
>  ltp/fsx.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 94 insertions(+), 2 deletions(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 47d3ee8..9dc1655 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -95,7 +95,8 @@ int			logcount = 0;	/* total ops */
>  #define OP_PUNCH_HOLE		6
>  #define OP_ZERO_RANGE		7
>  #define OP_COLLAPSE_RANGE	8
> -#define OP_MAX_FULL		9
> +#define OP_INSERT_RANGE	9
> +#define OP_MAX_FULL		10
>  
>  /* operation modifiers */
>  #define OP_CLOSEOPEN	100
> @@ -145,6 +146,7 @@ int     fallocate_calls = 1;            /* -F flag disables */
>  int     punch_hole_calls = 1;           /* -H flag disables */
>  int     zero_range_calls = 1;           /* -z flag disables */
>  int	collapse_range_calls = 1;	/* -C flag disables */
> +int	insert_range_calls = 1;		/* -i flag disables */
>  int 	mapped_reads = 1;		/* -R flag disables it */
>  int	fsxgoodfd = 0;
>  int	o_direct;			/* -Z */
> @@ -339,6 +341,14 @@ logdump(void)
>  						     lp->args[0] + lp->args[1])
>  				prt("\t******CCCC");
>  			break;
> +		case OP_INSERT_RANGE:
> +			prt("INSERT 0x%x thru 0x%x\t(0x%x bytes)",
> +			    lp->args[0], lp->args[0] + lp->args[1] - 1,
> +			    lp->args[1]);
> +			if (badoff >= lp->args[0] && badoff <
> +						     lp->args[0] + lp->args[1])
> +				prt("\t******CCCC");
> +			break;
>  		case OP_SKIPPED:
>  			prt("SKIPPED (no operation)");
>  			break;
> @@ -1012,6 +1022,59 @@ do_collapse_range(unsigned offset, unsigned length)
>  }
>  #endif
>  
> +#ifdef FALLOC_FL_INSERT_RANGE
> +void
> +do_insert_range(unsigned offset, unsigned length)
> +{
> +	unsigned end_offset;
> +	int mode = FALLOC_FL_INSERT_RANGE;
> +
> +	if (length == 0) {
> +		if (!quiet && testcalls > simulatedopcount)
> +			prt("skipping zero length insert range\n");
> +		log4(OP_SKIPPED, OP_INSERT_RANGE, offset, length);
> +		return;
> +	}
> +
> +	if ((loff_t)offset >= file_size) {
> +		if (!quiet && testcalls > simulatedopcount)
> +			prt("skipping insert range behind EOF\n");
> +		log4(OP_SKIPPED, OP_INSERT_RANGE, offset, length);
> +		return;
> +	}
> +
> +	log4(OP_INSERT_RANGE, offset, length, 0);
> +
> +	if (testcalls <= simulatedopcount)
> +		return;
> +
> +	end_offset = offset + length;
> +	if ((progressinterval && testcalls % progressinterval == 0) ||
> +	    (debug && (monitorstart == -1 || monitorend == -1 ||
> +		      end_offset <= monitorend))) {
> +		prt("%lu insert\tfrom 0x%x to 0x%x, (0x%x bytes)\n", testcalls,
> +			offset, offset+length, length);
> +	}
> +	if (fallocate(fd, mode, (loff_t)offset, (loff_t)length) == -1) {
> +		prt("insert range: %x to %x\n", offset, length);
> +		prterr("do_insert_range: fallocate");
> +		report_failure(161);
> +	}
> +
> +	memmove(good_buf + end_offset, good_buf + offset,
> +		file_size - offset);
> +	memset(good_buf + offset, '\0', length);
> +	file_size += length;
> +}
> +
> +#else
> +void
> +do_insert_range(unsigned offset, unsigned length)
> +{
> +	return;
> +}
> +#endif
> +
>  #ifdef HAVE_LINUX_FALLOC_H
>  /* fallocate is basically a no-op unless extending, then a lot like a truncate */
>  void
> @@ -1192,6 +1255,12 @@ test(void)
>  			goto out;
>  		}
>  		break;
> +	case OP_INSERT_RANGE:
> +		if (!insert_range_calls) {
> +			log4(OP_SKIPPED, OP_INSERT_RANGE, offset, size);
> +			goto out;
> +		}
> +		break;
>  	}
>  
>  	switch (op) {
> @@ -1244,6 +1313,21 @@ test(void)
>  		}
>  		do_collapse_range(offset, size);
>  		break;
> +	case OP_INSERT_RANGE:
> +		TRIM_OFF_LEN(offset, size, (maxfilelen - 1) - file_size);

I see a ton of "skipping insert beyond EOF" messages when I run fsx with
this patch that boil down to that we trim against the max allowable file
size increase rather than the current file size. I suspect the intent
here is to not limit the insert length based on the file size. That
makes sense, but that causes us to fail to mod the insert offset against
the file size and thus generate a ton more noise.

Could we either open code the trim to handle the offset/len correctly or
break up the macro in a way that facilitates doing so? For example, a
quick solution might be to create TRIM_OFF() and TRIM_LEN() based on the
associated code in TRIM_OFF_LEN(), redefine TRIM_OFF_LEN() to use the
new macros, and then the insert range code could do something like:

	TRIM_OFF(offset, file_size - 1);
	TRIM_LEN(offset, size, maxfilelen - file_size);
	...

Brian

> +		offset = offset & ~(block_size - 1);
> +		size = size & ~(block_size - 1);
> +		if (size == 0) {
> +			log4(OP_SKIPPED, OP_INSERT_RANGE, offset, size);
> +			goto out;
> +		}
> +		if (file_size + size > maxfilelen) {
> +			log4(OP_SKIPPED, OP_INSERT_RANGE, offset, size);
> +			goto out;
> +		}
> +
> +		do_insert_range(offset, size);
> +		break;
>  	default:
>  		prterr("test: unknown operation");
>  		report_failure(42);
> @@ -1307,6 +1391,9 @@ usage(void)
>  #ifdef FALLOC_FL_COLLAPSE_RANGE
>  "	-C: Do not use collapse range calls\n"
>  #endif
> +#ifdef FALLOC_FL_INSERT_RANGE
> +"	-i: Do not use insert range calls\n"
> +#endif
>  "	-L: fsxLite - no file creations & no file size changes\n\
>  	-N numops: total # operations to do (default infinity)\n\
>  	-O: use oplen (see -o flag) for every op (default random)\n\
> @@ -1493,7 +1580,7 @@ main(int argc, char **argv)
>  
>  	setvbuf(stdout, (char *)0, _IOLBF, 0); /* line buffered stdout */
>  
> -	while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHzCLN:OP:RS:WZ"))
> +	while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHzCiLN:OP:RS:WZ"))
>  	       != EOF)
>  		switch (ch) {
>  		case 'b':
> @@ -1599,6 +1686,9 @@ main(int argc, char **argv)
>  		case 'C':
>  			collapse_range_calls = 0;
>  			break;
> +		case 'i':
> +			insert_range_calls = 0;
> +			break;
>  		case 'L':
>  		        lite = 1;
>  			break;
> @@ -1758,6 +1848,8 @@ main(int argc, char **argv)
>  		zero_range_calls = test_fallocate(FALLOC_FL_ZERO_RANGE);
>  	if (collapse_range_calls)
>  		collapse_range_calls = test_fallocate(FALLOC_FL_COLLAPSE_RANGE);
> +	if (insert_range_calls)
> +		insert_range_calls = test_fallocate(FALLOC_FL_INSERT_RANGE);
>  
>  	while (numops == -1 || numops--)
>  		test();
> -- 
> 1.7.11-rc0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/