xfstests: introduce 280 for SEEK_DATA/SEEK_HOLE copy check

Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.

Signed-off-by Jie Liu<[email protected]>

---

src/seek_copy_tester.c new file +1 -0

new file mode 100644
index 0000000..ddf683f
 
+static size_t

src/seek_copy_tester.c new file +4 -0

+create_data_and_holes(int fd, size_t nr_total_bytes, off_t start_offset,
+		      uint64_t nr_skip_bytes, uint64_t nr_data_bytes,
+		      int wrote_hole_at_eof)
+{

src/seek_copy_tester.c new file +14 -0

+
+		off += (nr_skip_bytes + nr_data_bytes);
+		total -= off;
+	}
+
+	if (off<  nr_total_bytes) {
+		if (wrote_hole_at_eof) {
+			ret = ftruncate(fd, nr_total_bytes);
+			if (ret<  0) {
+				error("truncate source file to %lld bytes failed as %s",
+				       (long long)nr_total_bytes, strerror(errno));
+			}
+		}
+

Quoted 12 lines · Jeff Liu

Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.

Signed-off-by Jie Liu<[email protected]>

---

src/seek_copy_tester.c new file +0 -0

new file mode 100644
index 0000000..ddf683f

src/seek_copy_tester.c line 111

Quoted 11 lines

src/seek_copy_tester.c new file +11 -0

+static size_t
+full_write(int fd, const void *buf, size_t count)
+{
+    size_t total = 0;
+    const char *ptr = (const char *) buf;
+
+    while (count>  0) {
+        ssize_t n = write(fd, ptr, count);
+        if (n<  0) {
+            if (errno == EINTR)
+                continue;

Wouldn't you want to stop the write loop if interrupted?

src/seek_copy_tester.c line 117

Quoted 7 lines

src/seek_copy_tester.c new file +6 -0

+            error("full_write() failed as %s", strerror(errno));
+            break;
+        }
+
+        if (n == 0) {
+            error("full_write() ZERO bytes transferred");

You can tell them how many (total) bytes were written before the error. Same in full_pwrite()

Quoted 8 lines

src/seek_copy_tester.c new file +7 -0

+create_data_and_holes(int fd, size_t nr_total_bytes, off_t start_offset,
+              uint64_t nr_skip_bytes, uint64_t nr_data_bytes,
+              int wrote_hole_at_eof)
+{
+    int ret = 0;
+    off_t total = nr_total_bytes;
+    off_t data_len = nr_data_bytes;

src/seek_copy_tester.c

Quoted 10 lines

src/seek_copy_tester.c new file +7 -0

+    off_t off = start_offset;
+    char buf[BUF_SIZE];
+
+    memset(buf, 'A', sizeof(buf));
+
 
 
+    total -= start_offset;
 
+    while (total>  0) {

+ data_len = nr_data_bytes; /* see below */

src/seek_copy_tester.c line 242

Quoted 2 lines

src/seek_copy_tester.c new file +2 -0

+        do {
+            size_t nr_write = MIN(total, BUF_SIZE);

There are 2 dependencies:
1) The last nr_data_bytes does not exceed total (outer loop)
2) The remaining data size (data_len) could be less than BUF_SIZE (if data_len is not a multiple of BUF_SIZE for example).

src/seek_copy_tester.c

Quoted 9 lines

src/seek_copy_tester.c new file +9 -0

+            if (full_pwrite(fd, buf, nr_write, off) != nr_write) {
+                error("full_pwrite() failed as %s", strerror(errno));
+                ret = -1;
+                goto out;
+            }
+
+            off += nr_write;
+            data_len -= nr_write;
+        } while (data_len>  0);

data_len is <= 0 so the inner loop will execute only once if you have more than one chunk of data.

Quoted 14 lines

src/seek_copy_tester.c new file +14 -0

+
+        off += (nr_skip_bytes + nr_data_bytes);
+        total -= off;
+    }
+
+    if (off<  nr_total_bytes) {
+        if (wrote_hole_at_eof) {
+            ret = ftruncate(fd, nr_total_bytes);
+            if (ret<  0) {
+                error("truncate source file to %lld bytes failed as %s",
+                       (long long)nr_total_bytes, strerror(errno));
+            }
+        }
+

src/seek_copy_tester.c line 275

Quoted 10 lines

src/seek_copy_tester.c new file +10 -0

+        ret = write_zeros(fd, nr_total_bytes - off);
+        if (ret<  0) {
+            error("write_zeros to end of file failed as %s",
+                   strerror(errno));
+        }
+    }
+
+out:
+    return ret;
+}

It is shaping up nicely.

--Mark

Hi Mark,

Quoted 14 lines · Mark Tinguely

Quoted 12 lines · Jeff Liu

Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.

Signed-off-by Jie Liu<[email protected]>

---

src/seek_copy_tester.c new file +0 -0

new file mode 100644
index 0000000..ddf683f

Quoted 13 lines

src/seek_copy_tester.c line 111

Quoted 11 lines

src/seek_copy_tester.c new file +11 -0

+static size_t
+full_write(int fd, const void *buf, size_t count)
+{
+    size_t total = 0;
+    const char *ptr = (const char *) buf;
+
+    while (count>   0) {
+        ssize_t n = write(fd, ptr, count);
+        if (n<   0) {
+            if (errno == EINTR)
+                continue;

Wouldn't you want to stop the write loop if interrupted?

As this routine was called "full_write" which means it is expect to write as much as it can. So I would keep retrying if the process was interrupted by EINTR.
Would you please give some opinions whether this approach is not suitable in this circumstance?

Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.

Signed-off-by Jie Liu <[email protected]>

280 new file +5 -0

+# this is intended to test data buffer lookup for DIRTY pages.
+# verify results:
+# 1. file size is identical.
+# 2. perform cmp(1) to compare SRC and DEST file byte by byte.
+test01()

Quoted 3 lines · Jeff Liu

Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.

Signed-off-by Jie Liu <[email protected]>

This has the same problems with $seq.out as 279, so I won't repeat them here.

.....

280 line 59

Quoted 10 lines

280 new file +10 -0

+_cleanup()
+{
+	rm -f $src $dest
+}
+
+# seek_copy_test_01()
+# create a 100Mytes file in preallocation mode.
+# fallocate offset start from 0.
+# the first data extent offset start from 80991, write 4Kbytes,
+# and then skip 195001 bytes for next write.

Oh, man, you didn't write a program to do this, do you?

This is what
xfs_io is for - to create arbitary file configurations as quickly as you can type them. Then all you need is a simple program that copies the extents, and the test can check everything else.

Quoted 6 lines

280 new file +5 -0

+# this is intended to test data buffer lookup for DIRTY pages.
+# verify results:
+# 1. file size is identical.
+# 2. perform cmp(1) to compare SRC and DEST file byte by byte.
+test01()

280 line 74

Quoted 10 lines

280 new file +10 -0

+{
+	rm -f $src $dest
+
+	$here/src/seek_copy_tester -P -O 0 -L 100m -s 80991 -k 195001 -l 4k $src $dest
+
+	test $(stat --printf "%s" $src) = $(stat --printf "%s" $dest) ||
+		echo "TEST01: file size check failed" >> $seq.out
+
+	cmp $src $dest						      ||
+		echo "TEST01: file bytes check failed" >> $seq.out

A quick hack (untested) to replace this file creation with xfs_io would be:

test01()

{
	write_cmd="-c \"truncate 0\" -c \"falloc 0 100m\""
	for i in `seq 0 1 100`; do
		offset=$((80991 + $i * 195001))
		write_cmd="$write_cmd -c \"pwrite $offset 4k\""
	done
	xfs_io -F -f $write_cmd $src

	$here/scr/sparse_cp $src $dst
	stat --printf "%s\n" $src $dst
	cmp $src $dst >> $seq.out || _fail "file bytes check failed"
}

280 line 83

Quoted 11 lines

280 new file +9 -0

+}
+
+# seek_copy_test_02()
+# create a 100Mytes file in preallocation mode.
+# fallocate offset start from 0.
+# the first data extent offset start from 0, write 16Kbytes,
+# and then skip 8Mbytes for next write.
+# Try flushing DIRTY pages to WRITEBACK mode, this is intended to
+# test data buffer lookup in WRITEBACK pages.

There's no guarantee that that the seeks will occur while the pages are in the writeback. It's entirely dependent on IO latency - writing 16k of data to a disk cache will take less time than it takes to go back up into userspace and start the sparse copy. Indeed, i suspect that the 16x16k IOs that this tes does will fit all into that category even on basic SATA configs....

Also, you could the fadvise command in xfs_io to do this, as POSIX_FADV_DONTNEED will trigger async writeback -it will then skip invalidation of pages under writeback so they will remain in the cache. i.e. '-c "fadvise -d 0 100m"'

Ideally, we should add all the different sync methods to an xfs_io command...

280 line 104

Quoted 4 lines

280 new file +3 -0

+# the first data extent offset start from 512, write 4Kbytes,
+# and then skip 1Mbytes for next write.
+# don't make holes at the end of file.

I'm not sure what this means - you always write zeros at the end of file, and the only difference is that "make holes at EOF" does an ftruncate to the total size before writing zeros up to it. It appears to me like you end up with the same file size and shape either way....

280.out line 1

Quoted 5 lines

280.out new file +1 -0

@@ -0,0 +1 @@
+QA output created by 280

Normally we echo "silence is golden" to the output file in this case of no real output to indicate that this empty output file is intentional.

Cheers,

Dave.

Quoted 9 lines · Dave Chinner

Quoted 3 lines · Jeff Liu

Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.

Signed-off-by Jie Liu <[email protected]>

This has the same problems with $seq.out as 279, so I won't repeat them here.

.....

Quoted 12 lines

Review

Quoted 10 lines

(quoted diff) +10 -0

+_cleanup()
+{
+	rm -f $src $dest
+}
+
+# seek_copy_test_01()
+# create a 100Mytes file in preallocation mode.
+# fallocate offset start from 0.
+# the first data extent offset start from 80991, write 4Kbytes,
+# and then skip 195001 bytes for next write.

Oh, man, you didn't write a program to do this, do you?

Unfortunately, I have already included file creation at seek_copy_tester :(

Quoted 4 lines

This is what
xfs_io is for - to create arbitary file configurations as quickly as you can type them. Then all you need is a simple program that copies the extents, and the test can check everything else.

Yes, xfs_io is pretty cool, and it really convenient for file creation for XFS.

I wrote it(create_data_and_holes()) in seek_copy_tester since I'd make it as a general SEEK_DATA/SEEK_HOLE tester for other file systems without this utility too.

Quoted 20 lines

Review

Quoted 7 lines

(quoted diff) +7 -0

+# seek_copy_test_02()
+# create a 100Mytes file in preallocation mode.
+# fallocate offset start from 0.
+# the first data extent offset start from 0, write 16Kbytes,
+# and then skip 8Mbytes for next write.
+# Try flushing DIRTY pages to WRITEBACK mode, this is intended to
+# test data buffer lookup in WRITEBACK pages.

There's no guarantee that that the seeks will occur while the pages are in the writeback. It's entirely dependent on IO latency - writing 16k of data to a disk cache will take less time than it takes to go back up into userspace and start the sparse copy. Indeed, i suspect that the 16x16k IOs that this tes does will fit all into that category even on basic SATA configs....

Also, you could the fadvise command in xfs_io to do this, as POSIX_FADV_DONTNEED will trigger async writeback -it will then skip invalidation of pages under writeback so they will remain in the cache. i.e. '-c "fadvise -d 0 100m"'

Quoted 2 lines

Ideally, we should add all the different sync methods to an xfs_io command...

Thanks again for the detained info.
It's definitely depending on the IO latency to test cover those page status conversion. I have verified the old patch with page probe routine on my laptop SATA disk controller, but not tried against other faster controllers. If we agree to make it as a general tester, maybe I can try to implement it by referring to xfs_io fadvise, I guess it use posix_fadvise(2), will check it later.

Quoted 9 lines

Review

Quoted 3 lines

(quoted diff) +3 -0

+# the first data extent offset start from 512, write 4Kbytes,
+# and then skip 1Mbytes for next write.
+# don't make holes at the end of file.

I'm not sure what this means - you always write zeros at the end of file, and the only difference is that "make holes at EOF" does an ftruncate to the total size before writing zeros up to it. It appears to me like you end up with the same file size and shape either way....

Oops! this is a code bug. I want to create a hole at EOF if possible when "-E(wrote_hole_at_eof)" option was specified. It can be fixed as below FIXME:

Quoted 31 lines · Jeff Liu

Quoted 9 lines · Dave Chinner

Quoted 3 lines · Jeff Liu

Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.

Signed-off-by Jie Liu <[email protected]>

This has the same problems with $seq.out as 279, so I won't repeat them here.

.....

Quoted 12 lines

Review

Quoted 10 lines

(quoted diff) +10 -0

+_cleanup()
+{
+	rm -f $src $dest
+}
+
+# seek_copy_test_01()
+# create a 100Mytes file in preallocation mode.
+# fallocate offset start from 0.
+# the first data extent offset start from 80991, write 4Kbytes,
+# and then skip 195001 bytes for next write.

Oh, man, you didn't write a program to do this, do you?

Unfortunately, I have already included file creation at seek_copy_tester :(

Quoted 4 lines

This is what
xfs_io is for - to create arbitary file configurations as quickly as you can type them. Then all you need is a simple program that copies the extents, and the test can check everything else.

Quoted 1 line

Yes, xfs_io is pretty cool, and it really convenient for file creation for XFS.

xfs_io is filesystem agnostic. Currently it needs the "-F" flag to tell it to work on non-xfs filesystems, but Eric posted patches a couple of days ago to remove that (i.e to automatically detect XFS filesystems and enable all the xfs specific stuff).

Quoted 3 lines

I wrote it(create_data_and_holes()) in seek_copy_tester since I'd make it as a general SEEK_DATA/SEEK_HOLE tester for other file systems without this utility too.

xfs_io is used all throughout xfstests in generic tests. Just look at common.punch::_test_generic_punch as an example. That function uses xfs_io to test the different methods of perallocation and hole punching supported by a bunch of different filesystems in 3 different tests. IOWs, the generic tests use fallocate and the XFS specific tests use XFS ioctls, but all tests use xfs_io to run the commands....

Quoted 21 lines

Quoted 21 lines

Review

Quoted 8 lines

(quoted diff) +7 -0

+# seek_copy_test_02()
+# create a 100Mytes file in preallocation mode.
+# fallocate offset start from 0.
+# the first data extent offset start from 0, write 16Kbytes,
+# and then skip 8Mbytes for next write.
+# Try flushing DIRTY pages to WRITEBACK mode, this is intended to
+# test data buffer lookup in WRITEBACK pages.

There's no guarantee that that the seeks will occur while the pages are in the writeback. It's entirely dependent on IO latency - writing 16k of data to a disk cache will take less time than it takes to go back up into userspace and start the sparse copy. Indeed, i suspect that the 16x16k IOs that this tes does will fit all into that category even on basic SATA configs....

Also, you could the fadvise command in xfs_io to do this, as POSIX_FADV_DONTNEED will trigger async writeback -it will then skip invalidation of pages under writeback so they will remain in the cache. i.e. '-c "fadvise -d 0 100m"'

Quoted 8 lines

Quoted 2 lines

Ideally, we should add all the different sync methods to an xfs_io command...

Thanks again for the detained info.
It's definitely depending on the IO latency to test cover those page status conversion. I have verified the old patch with page probe routine on my laptop SATA disk controller, but not tried against other faster controllers. If we agree to make it as a general tester, maybe I can try to implement it by referring to xfs_io fadvise, I guess it use posix_fadvise(2), will check it later.

Yes, it uses posix_fadvise64().

As it is, I spent 15 minutes adding support for sync_file_range() to xfs_io. The patch is attached below.

Quoted 11 lines

Quoted 10 lines

Review

Quoted 4 lines

(quoted diff) +3 -0

+# the first data extent offset start from 512, write 4Kbytes,
+# and then skip 1Mbytes for next write.
+# don't make holes at the end of file.

I'm not sure what this means - you always write zeros at the end of file, and the only difference is that "make holes at EOF" does an ftruncate to the total size before writing zeros up to it. It appears to me like you end up with the same file size and shape either way....

Quoted 2 lines

Oops! this is a code bug. I want to create a hole at EOF if possible when "-E(wrote_hole_at_eof)" option was specified. It can be fixed as below FIXME:

Yes, that'd work ;)

Cheers,

Dave.

Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.

Signed-off-by Jie Liu<[email protected]>

src/seek_copy_tester.c new file +3 -0

+static int
+do_extent_copy(int src_fd, int dest_fd, off_t data_off, off_t hole_off)
+{

src/seek_copy_tester.c new file +5 -0

+static int
+copy_extents(int src_fd, int dest_fd, off_t src_total_size)
+{
+	int ret = 0;
+	unsigned int i = 0;

Quoted 10 lines · Jeff Liu

Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.

Signed-off-by Jie Liu<[email protected]>

src/seek_copy_tester.c new file +3 -0

+static int
+do_extent_copy(int src_fd, int dest_fd, off_t data_off, off_t hole_off)
+{

...

src/seek_copy_tester.c line 308

Quoted 4 lines

src/seek_copy_tester.c new file +4 -0

+
+    while (len>  0) {
+        memset(buf, 0, sizeof(buf));
+        ssize_t nr_read = read(src_fd, buf, BUF_SIZE);

#RCJ The memset above is unnecessary.

Quoted 5 lines

src/seek_copy_tester.c new file +5 -0

+static int
+copy_extents(int src_fd, int dest_fd, off_t src_total_size)
+{
+    int ret = 0;
+    unsigned int i = 0;

...

src/seek_copy_tester.c

Quoted 3 lines

src/seek_copy_tester.c new file +3 -0

+        ++i;
+        seek_start = hole_pos;
+    } while (seek_start<  src_total_size);

#RCJ i is unused.

Other than the comments above, this version of the test looks good.

Reviewed-by Rich Johnston<[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs