dm-thinp feature request: skip allocation on writes of all zeroes

Hello all,

We have been testing dm-thinp for the last couple years and it has been stable for the most part.

I have always wondered, does LVM ignore writes with blocks of all zeroes when the destination address is unallocated? From experience, it does not appear that this is so (or I cannot find the config option). This would reduce IOs, particularily when importing VM disk images from other sources. Currently, we must import, allocate lots of zeroes, then fstrim after the fact.

Hello all,

We have been testing dm-thinp for the last couple years and it has been stable for the most part.

I have always wondered, does LVM ignore writes with blocks of all zeroes when the destination address is unallocated? From experience, it does not appear that this is so (or I cannot find the config option). This would reduce IOs, particularily when importing VM disk images from other sources. Currently, we must import, allocate lots of zeroes, then fstrim after the fact.

I have paged through the dm-thinp code, but I am not quite certain what the best place to add this logic would be such that the IO is completed and the allocation is not performed.

I would be happy to work on a patch, but I need a little bit of direction:

What would it take to add this as a feature option to the dmsetup table string?

Where in the dm-thinp code would this best be addressed?

Quoted 6 lines

I have always wondered, does LVM ignore writes with blocks of all zeroes when the destination address is unallocated? From experience, it does not appear that this is so (or I cannot find the config option). This would reduce IOs, particularily when importing VM disk images from other sources. Currently, we must import, allocate lots of zeroes, then fstrim after the fact.

And in your example fstrim wouldn't do anything for you. [...] fstrim is only useful to reclaim blocks that are no longer used.

The zeroed data in the image is still valid isn't it?

We don't inspect the data being written to infer WRITE SAME-like action.

This patch skips all-zero writes to unallocated blocks of dm-thinp volumes.

Unallocated zero-writes are 70x faster and never allocate space in this test:

        # dd if=/dev/zero of=/dev/test/test1 bs=1M count=1024
        1073741824 bytes (1.1 GB) copied, 0.794343 s, 1.4 GB/s

Without the patch, zero-writes allocate space and hit the disk:

        # dd if=/dev/zero of=/dev/test/test1 bs=1M count=1024
        1073741824 bytes (1.1 GB) copied, 53.8064 s, 20.0 MB/s

For the test below, notice the allocation difference for thin volumes test1 and test2 (after dd if=test1 of=test2), even though they have the same md5sum:
LV VG Attr LSize Pool Origin Data%
test1 test Vwi-a-tz-- 4.00g thinp 22.04
test2 test Vwi-a-tz-- 4.00g thinp 18.33

An additional 3.71% of space was saved by the patch, and so were the ~150MB of (possibly random) IOs that would have hit disk, not to mention reads that now bypass the disk since they are unallocated. We also save the metadata overhead of ~2400 allocations when calling provision_block().

        # lvcreate -T test/thinp -L 5G
        # lvcreate -T test/thinp -V 4G -n test1
        # lvcreate -T test/thinp -V 4G -n test2

Simple ext4+kernel tree extract test:

First prepare two dm-thinp volumes test1 and test2 of equal size. First mkfs.ext4 /dev/test/test1 without the patch and then mount and extract 3.17.4's source tree onto the test1 filesystem, and unmount

Next, install patched dm_thin_pool.ko, then dd test1 over test2 and verify checksums:

        # dd if=/dev/test/test1  of=/dev/test/test2 bs=1M
        # md5sum /dev/test/test?
        b210f032a6465178103317f3c40ab59f  /dev/test/test1
        b210f032a6465178103317f3c40ab59f  /dev/test/test2
Yes, they match!

Signed-off-by Eric Wheeler <[email protected]>

---
Resending the patch as it was malformed on the first try.

drivers/md/dm-thin.c +6 -0

+/* return true if bio data contains all 0x00's */
+bool bio_all_zeros(struct bio *bio) +{
+	unsigned long flags;
+	struct bio_vec bv;
+	struct bvec_iter iter;
+

drivers/md/dm-thin.c +18 -0

+
+		for (i = 0; i < count; i++) {
+			if (*p)	{
+				allzeros = false;
+				break;
+			}
+			p++;
+		}
+
+		bvec_kunmap_irq(data, &flags);
+
+		if (likely(!allzeros))
+				break;
+	}
+
+	return allzeros;
+}
+
 static void provision_block(struct thin_c *tc, struct bio *bio, dm_block_t block,
 			    struct dm_bio_prison_cell *cell)
 {

@@ -1258,6 +1294,15 @@ static void provision_block(struct thin_c *tc, struct bio *bio, dm_block_t block
 		return;
 	}

drivers/md/dm-thin.c static void process_shared_bio(struct thin_c *tc, struct bio *bio, · line 1231

Quoted 7 lines

drivers/md/dm-thin.c +0 -0

index fc9c848..71dd545 100644
@@ -1230,6 +1230,42 @@ static void process_shared_bio(struct thin_c *tc, struct bio *bio,
 	}
 }

A helper like this really belongs in block/bio.c:

Quoted 13 lines

drivers/md/dm-thin.c +13 -0

+/* return true if bio data contains all 0x00's */
+bool bio_all_zeros(struct bio *bio) +{
+	unsigned long flags;
+	struct bio_vec bv;
+	struct bvec_iter iter;
+
+	char *data;
+	uint64_t *p;
+	int i, count;
+ +	bool allzeros = true;
+
+	bio_for_each_segment(bv, bio, iter) {
+		data = bvec_kmap_irq(&bv, &flags);

drivers/md/dm-thin.c static void process_shared_bio(struct thin_c *tc, struct bio *bio, · line 1247

Quoted 3 lines

drivers/md/dm-thin.c +3 -0

+
+		p = (uint64_t*)data;
+		count = bv.bv_len / sizeof(uint64_t);

Addressing a bio's contents in terms of uint64_t has the potential to access beyond bv.bv_len (byte addressing vs 64bit addressing). I can see you were just looking to be more efficient about checking the bios' contents but I'm not convinced it would always be safe.

Actually, given your: count = bv.bv_len / sizeof(uint64_t);

My immediate concern was it would've truncated any partial contents of the bio_vec beyond the last uint64_t boundary. Leading to possible false positive return from the function (because some contents weren't checked).

--
lvm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/lvm-devel

From: Mike Snitzer <[email protected]>
Date: Thu, 4 Dec 2014 10:18:32 -0500
Subject: [PATCH] dm thin: optimize away writing all zeroes to unprovisioned blocks

Introduce bio_is_zero_filled() and use it to optimize away writing all zeroes to unprovisioned blocks. Subsequent reads to the associated unprovisioned blocks will be zero filled.

Signed-off-by Mike Snitzer <[email protected]>

Cc Eric Wheeler <[email protected]>

Cc Jens Axboe <[email protected]>

On Thu, Dec 04 2014 at 10:33am -0500,
Mike Snitzer <[email protected]> wrote:
In testing this patch it is now quite clear that this change seriously impacts test coverage in the device-mapper-test-suite because dmts' wipe_device() uses /dev/zero as the ifile for dd to write to the disk. As such, with this patch all tests expecting to see provisioned blocks as a side-effect of wipe_device now fail (e.g. DiscardQuickTests).

So this change won't go upstream until full test coverage can be restored in dmts. I'll see what I can come up with but it is low priority.

BTW, this also makes me wonder if this change will be extremely unintuitive to existing or future user of DM thinp. So much so that it might be best to require a new thin-pool feature flag to enable this optimization.

Mike

--
lvm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/lvm-devel

Here is a revised patch that is more like I'd expect to land upstream. Jens are you OK with us adding bio_is_zero_filled to block/bio.c? If so should I split it out as a separate patch for you to pick up or just carry it as part of the patch that lands in linux-dm.git?

Quoted 5 lines · Mike Snitzer

Here is a revised patch that is more like I'd expect to land upstream. Jens are you OK with us adding bio_is_zero_filled to block/bio.c? If so should I split it out as a separate patch for you to pick up or just carry it as part of the patch that lands in linux-dm.git?

Yeah I'm fine with adding that helper.

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled, but with the last byte not being a zero?

Quoted 3 lines

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled, but with the last byte not being a zero?

I haven't tested that yet, but I share your concern. Does fio offer the ability to slap down a specific pattern like this? If not is there a logical place to extend fio to enable this?

--
lvm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/lvm-devel

Quoted 3 lines · Mike Snitzer

Quoted 3 lines

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled, but with the last byte not being a zero?

The additional overhead of worst-case should be (nearly) equal to the simplest test case of dd if=/dev/zero of=/dev/thinp/vol. In my testing that was 1.4GB/s within KVM on an Intel Xeon(R) CPU E3-1230 V2 @ 3.30GHz.

I could see where really fast storage that can obtain 1.4gb/s might notice a performance regression---but for most use cases that is quite fast.

Quoted 8 lines · Eric Wheeler

Quoted 3 lines · Mike Snitzer

Quoted 3 lines

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled, but with the last byte not being a zero?

The additional overhead of worst-case should be (nearly) equal to the simplest test case of dd if=/dev/zero of=/dev/thinp/vol. In my testing that was 1.4GB/s within KVM on an Intel Xeon(R) CPU E3-1230 V2 @ 3.30GHz.

That seems way too slow for checking if it's zero or not... Memory bandwidth should be way higher than that. The line above, was that what you ran? How does it look with bs=4k or higher?

read : io=12233MB, bw=1432.7MB/s, iops=22922, runt= 8539msec

Also, attached is the patch that supports uintptr_t word sized 0-checks.

Quoted 11 lines · Jens Axboe

Quoted 9 lines · Eric Wheeler

Quoted 3 lines · Mike Snitzer

Quoted 3 lines

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled, but with the last byte not being a zero?

The additional overhead of worst-case should be (nearly) equal to the simplest test case of dd if=/dev/zero of=/dev/thinp/vol. In my testing that was 1.4GB/s within KVM on an Intel Xeon(R) CPU E3-1230 V2 @ 3.30GHz.

Quoted 3 lines

That seems way too slow for checking if it's zero or not... Memory bandwidth should be way higher than that. The line above, was that what you ran? How does it look with bs=4k or higher?

In userspace I can get ~12GB/s, so I think the algorithm is sound. dd might not be the right tool for this.

Quoted 1 line

read : io=12233MB, bw=1432.7MB/s, iops=22922, runt= 8539msec

Can you suggest the right fio commandline to test sequential writes if all zeros? I tried --zero_buffers but couldn't get it to write zeros, writes kept going to disk.

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled,

Quoted 2 lines · Mike Snitzer

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled,

Jens, thank you for your help w/ fio for generating zeroed writes! Clearly fio is superior to dd as a sequential benchmarking tool; I was actually able to push on the system's memory bandwidth.

Results:

I hacked block/loop.c and md/dm-thin.c to always call bio_is_zero_filled() and then complete without writing to disk, regardless of the return value from bio_is_zero_filled(). In loop.c this was done in do_bio_filebacked(), and for dm-thin.c this was done within provision_block().

This allows us to compare the performance difference between the simple loopback block device driver vs the more complex dm-thinp implementation just prior to block allocation. These benchmarks give us a sense of how performance differences relate between bio_is_zero_filled() and block device implementation complexity, in addition to the raw performance of bio_is_zero_filled in best- and worst-case scenarios.

Since we always complete without writing after the call to bio_is_zero_filled, regardless of the bio's content (all zeros or not), we can benchmark the difference in the common use case of random data, as well as the edge case of skipping writes for bio's that contain all zeros when writing to unallocated space of thin-provisioned volumes.

These benchmarks were performed under KVM, so expect them to be lower bounds due to overhead. The hardware is a Intel(R) Xeon(R) CPU E3-1230 V2 @ 3.30GHz. The VM was allocated 4GB of memory with 4 cpu cores.

Benchmarks were performed using fio-2.1.14-33-gf8b8f
--name=writebw
--rw=write
--time_based
--runtime=7 --ramp_time=3
--norandommap
--ioengine=libaio
--group_reporting
--direct=1
--bs=1m
--filename=/dev/X
--numjobs=Y

Random data was tested using:

   --zero_buffers=0 --scramble_buffers=1

Zeroed data was tested using:

   --zero_buffers=1 --scramble_buffers=0

Values below are from aggrb.

dm-thinp (MB/s) loopback (MB/s) loop faster by factor of
==============+======================================================

random jobs=4 | 18496.0          33522.0           1.68x
zeros  jobs=4 |  8119.2           9767.2           1.20x
==============+======================================================
random jobs=1 |  7330.5          12330.0           1.81x
zeros  jobs=1 |  4965.2           6799.9           1.11x

Quoted 57 lines · Eric Wheeler

Quoted 2 lines · Mike Snitzer

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled,

Jens, thank you for your help w/ fio for generating zeroed writes! Clearly fio is superior to dd as a sequential benchmarking tool; I was actually able to push on the system's memory bandwidth.

Results:

I hacked block/loop.c and md/dm-thin.c to always call bio_is_zero_filled()
and then complete without writing to disk, regardless of the return value from bio_is_zero_filled(). In loop.c this was done in do_bio_filebacked(), and for dm-thin.c this was done within provision_block().

This allows us to compare the performance difference between the simple loopback block device driver vs the more complex dm-thinp implementation just prior to block allocation. These benchmarks give us a sense of how performance differences relate between bio_is_zero_filled() and block device implementation complexity, in addition to the raw performance of bio_is_zero_filled in best- and worst-case scenarios.

Since we always complete without writing after the call to bio_is_zero_filled, regardless of the bio's content (all zeros or not), we
can benchmark the difference in the common use case of random data, as well as the edge case of skipping writes for bio's that contain all zeros when writing to unallocated space of thin-provisioned volumes.

These benchmarks were performed under KVM, so expect them to be lower bounds due to overhead. The hardware is a Intel(R) Xeon(R) CPU E3-1230 V2
@ 3.30GHz. The VM was allocated 4GB of memory with 4 cpu cores.

Benchmarks were performed using fio-2.1.14-33-gf8b8f
--name=writebw
--rw=write
--time_based
--runtime=7 --ramp_time=3
--norandommap
--ioengine=libaio
--group_reporting
--direct=1
--bs=1m
--filename=/dev/X
--numjobs=Y

Random data was tested using:

   --zero_buffers=0 --scramble_buffers=1

Zeroed data was tested using:

   --zero_buffers=1 --scramble_buffers=0

Values below are from aggrb.

Quoted 8 lines

dm-thinp (MB/s) loopback (MB/s) loop faster by
factor of
==============+======================================================

random jobs=4 | 18496.0          33522.0           1.68x
zeros  jobs=4 |  8119.2           9767.2           1.20x
==============+======================================================
random jobs=1 |  7330.5          12330.0           1.81x
zeros  jobs=1 |  4965.2           6799.9           1.11x

This looks more reasonable in terms of throughput.

One major worry here is that checking every write is blowing your cache, so you could have a major impact on performance in general. Even for O_DIRECT writes, you are now accessing the memory. Have you looked into doing non-temporal memory compares instead? I think that would be the way to go.

Quoted 9 lines

Quoted 8 lines

dm-thinp (MB/s) loopback (MB/s) loop faster by

==============+======================================================

random jobs=4 | 18496.0          33522.0           1.68x
zeros  jobs=4 |  8119.2           9767.2           1.20x
==============+======================================================
random jobs=1 |  7330.5          12330.0           1.81x
zeros  jobs=1 |  4965.2           6799.9           1.11x

Quoted 7 lines

This looks more reasonable in terms of throughput.

One major worry here is that checking every write is blowing your cache, so you could have a major impact on performance in general. Even for O_DIRECT writes, you are now accessing the memory. Have you looked into doing non-temporal memory compares instead? I think that would be the way to go.

So I found your patch in the thread. For each vector, use memcmp() instead and hope it does the right thing. You can compare with empty_zero_page. That should drastically cut down on the amount of hand rolled code you have in bio_is_zero_filled() at the moment.

Introduce bio_is_zero_filled() and use it to optimize away writing all zeroes to unprovisioned blocks. Subsequent reads to the associated unprovisioned blocks will be zero filled.

Single-thread performance evaluates zero bio's at ~8137MB/s under KVM on a Xeon E3-1230. Zero-checks attempt to minimize cache effects on non-zero data. bio_is_zero_filled works with unaligned bvec data. (Using memcmp and comparing against the zero page is ~5x slower, so this implementation was optimized to increase pipelined exectution.)

The pool_feature pf.skip_zero_writes is implemented and configurable at creation time or via table reload.

To test we prepare two dm-thinp volumes test1 and test2 of equal size. We format test1 without the patch, mount, and extract the Linux source tree onto the test1 filesystem, and unmount. Finally, we activate skip_zero_writes, dd test1 over test2, and verify checksums:

        b210f032a6465178103317f3c40ab59f  /dev/test/test1
        b210f032a6465178103317f3c40ab59f  /dev/test/test2

Notice the allocation difference for thin volumes test1 and test2 (after dd if=test1 of=test2), even though they have the same md5sum:
LV VG Attr LSize Pool Origin Data%
test1 test Vwi-a-tz-- 4.00g thinp 22.04
test2 test Vwi-a-tz-- 4.00g thinp 18.33

An additional 3.71% of space was saved by the patch, and so were the ~150MB of (possibly random) IOs that would have hit disk, not to mention reads that now bypass the disk since they are unallocated. We also save the metadata overhead of ~2400 provision_block() allocations.

Signed-off-by Eric Wheeler <[email protected]>

Cc Jens Axboe <[email protected]>

Cc [email protected]

---
 Documentation/device-mapper/thin-provisioning.txt |    2 +
 block/bio.c                                       |  103 +++++++++++++++++++++
 drivers/md/dm-thin.c                              |   27 ++++++
 include/linux/bio.h                               |    1 +
 4 files changed, 133 insertions(+), 0 deletions(-)

http://www.globallinuxsecurity.pro/

Documentation/device-mapper/thin-provisioning.txt +2 -0

index 2f51735..7304ccf 100644
@@ -266,6 +266,8 @@ i) Constructor
 
       error_if_no_space: Error IOs, instead of queueing, if no space.
 
+	  enable_zero_writes: Enable all-zero writes to unallocated blocks. 
+    
     Data block size must be between 64KB (128 sectors) and 1GB
     (2097152 sectors) inclusive.
 

block/bio.c +103 -0

index 8c2e55e..7e372b7 100644
@@ -511,6 +511,109 @@ void zero_fill_bio(struct bio *bio)
 }
 EXPORT_SYMBOL(zero_fill_bio);
 
+__attribute__((optimize("unroll-loops")))
+bool bio_is_zero_filled(struct bio *bio)
+{
+	struct bio_vec bv;
+	struct bvec_iter iter;
+	char *data, *p;
+	__uint128_t *p128;
+
+	unsigned i, count, left;
+	unsigned long flags;
+
+	/* Align to the data chunk size */
+	const int align_to = sizeof(__uint128_t);
+
+	p128 = NULL;
+	bio_for_each_segment(bv, bio, iter) {
+		data = bvec_kmap_irq(&bv, &flags);
+		p = data;
+		left = bv.bv_len;
+
+		if (unlikely( data == NULL ))
+			continue;
+
+		/* check unaligned bytes at the beginning of p, if any */
+		if (unlikely( ( (uintptr_t)p & (align_to-1) ) != 0 )) {
+			count = align_to - ( (uintptr_t)p & (align_to-1) );
+			for (i = 0; i < count; i++) {
+				if (*p) 
+					break;
+				p++;
+			}
+			if (i < count)
+				goto out_false;
+			left -= count;
+		}
+
+		/* we should be align_to-byte aligned now */
+		BUG_ON(unlikely( ((uintptr_t)p & (align_to-1) ) != 0 ));
+
+		p128 = (__uint128_t*)p;
+		count = left >> 9; /* count = left / (32*16) */
+
+		/* Quickly sample first, middle, and last values 
+		 * and exit early without hitting the loop if nonzero.
+		 * This reduces cache effects for non-zero data and has almost
+		 * no effective penalty since these values will probably be in 
+		 * cache when we test them below.
+		 */
+		if (likely(count) &&
+			   (p128[0] || p128[left>>5] || p128[(left>>4)-1]))
+			goto out_false;
+
+		/* We are using __uint128_t here, so 32*16=512 bytes per loop 
+		 * Writing out the 32 operations helps with pipelining. */
+		for (i = 0; i < count; i++) {
+			if (p128[ 0] | p128[ 1] | p128[ 2] | p128[ 3] |
+			    p128[ 4] | p128[ 5] | p128[ 6] | p128[ 7] |
+			    p128[ 8] | p128[ 9] | p128[10] | p128[11] |
+			    p128[12] | p128[13] | p128[14] | p128[15] |
+			    p128[16] | p128[17] | p128[18] | p128[19] |
+			    p128[20] | p128[21] | p128[22] | p128[23] |
+			    p128[24] | p128[25] | p128[26] | p128[27] |
+			    p128[28] | p128[29] | p128[30] | p128[31])
+				break;
+
+			p128 += 32;
+		}
+		if (i < count)
+			goto out_false;
+
+		left -= count << 9; /* left -= count * 32*16 */
+
+		/* check remaining unaligned values at the end, if any */
+		if (unlikely(left > 0))	{
+			p = (char*)p128;
+			for (i = 0; i < left; i++) {
+				if (*p)
+					break;
+				p++;
+			}
+			if (i < left)
+				goto out_false;
+
+			/* Fixup values for the BUG_ON checks below */
+			p128 = (__uint128_t*)p;
+			left = 0;
+		}
+
+		bvec_kunmap_irq(data, &flags);
+		BUG_ON(unlikely( left > 0 ));
+		BUG_ON(unlikely( data+bv.bv_len != (char*)p128 ));
+	}
+
+	return true;
+
+out_false:
+	if (likely(data != NULL))
+		bvec_kunmap_irq(data, &flags);
+
+	return false;
+}
+EXPORT_SYMBOL(bio_is_zero_filled);
+
 /**
  * bio_put - release a reference to a bio
  * @bio:   bio to release reference to

drivers/md/dm-thin.c +27 -0

index fc9c848..d0cebd2 100644
@@ -151,6 +151,7 @@ struct pool_features {
 	bool discard_enabled:1;
 	bool discard_passdown:1;
 	bool error_if_no_space:1;
+	bool skip_zero_writes:1;
 };
 
 struct thin_c;

@@ -1258,6 +1259,16 @@ static void provision_block(struct thin_c *tc, struct bio *bio, dm_block_t block
 		return;
 	}
 
+        /*
+	 *  Skip writes of all zeroes to unprovisioned blocks.
+	 */
+
+	if (tc->pool->pf.skip_zero_writes && bio_is_zero_filled(bio) ) {
+		cell_defer_no_holder(tc, cell);
+		bio_endio(bio, 0);
+		return;
+	}
+
 	r = alloc_data_block(tc, &data_block);
 	switch (r) {
 	case 0:

@@ -2063,6 +2074,7 @@ static void pool_features_init(struct pool_features *pf)
 	pf->discard_enabled = true;
 	pf->discard_passdown = true;
 	pf->error_if_no_space = false;
+	pf->skip_zero_writes = true;
 }
 
 static void __pool_destroy(struct pool *pool)

@@ -2310,6 +2322,9 @@ static int parse_pool_features(struct dm_arg_set *as, struct pool_features *pf,
 		else if (!strcasecmp(arg_name, "error_if_no_space"))
 			pf->error_if_no_space = true;
 
+		else if (!strcasecmp(arg_name, "enable_zero_writes"))
+			pf->skip_zero_writes = false;
+
 		else {
 			ti->error = "Unrecognised pool feature requested";
 			r = -EINVAL;

@@ -2393,6 +2408,7 @@ static dm_block_t calc_metadata_threshold(struct pool_c *pt)
  *	     no_discard_passdown: don't pass discards down to the data device
  *	     read_only: Don't allow any changes to be made to the pool metadata.
  *	     error_if_no_space: error IOs, instead of queueing, if no space.
+ *	     enable_zero_writes: Enable all-zero writes to unallocated blocks.
  */
 static int pool_ctr(struct dm_target *ti, unsigned argc, char **argv)
 {

@@ -2511,6 +2527,9 @@ static int pool_ctr(struct dm_target *ti, unsigned argc, char **argv)
 	}
 	ti->private = pt;
 
+	/* The pf skip_zero_writes is safe to change at any time */
+	pool->pf.skip_zero_writes = pf.skip_zero_writes;
+
 	r = dm_pool_register_metadata_threshold(pt->pool->pmd,
 						calc_metadata_threshold(pt),
 						metadata_low_callback,

@@ -2936,6 +2955,9 @@ static void emit_flags(struct pool_features *pf, char *result,
 
 	if (pf->error_if_no_space)
 		DMEMIT("error_if_no_space ");
+	
+	if (!pf->skip_zero_writes)
+		DMEMIT("enable_zero_writes ");
 }
 
 /*

@@ -3043,6 +3065,11 @@ static void pool_status(struct dm_target *ti, status_type_t type,
 		else
 			DMEMIT("queue_if_no_space ");
 
+		if (!pool->pf.skip_zero_writes)
+			DMEMIT("enable_zero_writes ");
+		else
+			DMEMIT("skip_zero_writes ");
+
 		break;
 
 	case STATUSTYPE_TABLE:

include/linux/bio.h +1 -0

index 5a64576..abb46f7 100644
@@ -419,6 +419,7 @@ extern struct bio *bio_copy_user_iov(struct request_queue *,
 				     int, int, gfp_t);
 extern int bio_uncopy_user(struct bio *);
 void zero_fill_bio(struct bio *bio);
+bool bio_is_zero_filled(struct bio *bio);
 extern struct bio_vec *bvec_alloc(gfp_t, int, unsigned long *, mempool_t *);
 extern void bvec_free(mempool_t *, struct bio_vec *, unsigned int);
 extern unsigned int bvec_nr_vecs(unsigned short idx);

Signature · 6 lines

1.7.1

--
lvm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/lvm-devel

Quoted 16 lines · Eric Wheeler

Quoted 14 lines · Jens Axboe

Quoted 9 lines · Eric Wheeler

Quoted 3 lines · Mike Snitzer

Quoted 3 lines

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled, but with the last byte not being a zero?

The additional overhead of worst-case should be (nearly) equal to the simplest test case of dd if=/dev/zero of=/dev/thinp/vol. In my testing that was 1.4GB/s within KVM on an Intel Xeon(R) CPU E3-1230 V2 @ 3.30GHz.

That seems way too slow for checking if it's zero or not... Memory bandwidth should be way higher than that. The line above, was that what you ran? How does it look with bs=4k or higher?

Quoted 2 lines

In userspace I can get ~12GB/s, so I think the algorithm is sound. dd might not be the right tool for this.

It's straight forward looping through the vectors, it can't really work any other way. But we need to figure out why it's so slow...

Quoted 6 lines

Quoted 2 lines

read : io=12233MB, bw=1432.7MB/s, iops=22922, runt= 8539msec

Can you suggest the right fio commandline to test sequential writes if all zeros? I tried --zero_buffers but couldn't get it to write zeros, writes kept going to disk.

zero_buffers=1
scramble_buffers=0

should get you all zeroes.

Quoted 8 lines · Jens Axboe

Quoted 5 lines · Mike Snitzer

Here is a revised patch that is more like I'd expect to land upstream. Jens are you OK with us adding bio_is_zero_filled to block/bio.c? If so should I split it out as a separate patch for you to pick up or just carry it as part of the patch that lands in linux-dm.git?

Yeah I'm fine with adding that helper.

Good to know, thanks.

Quoted 3 lines

I do wonder what the performance impact is on this for dm. Have you tried a (worst case) test of writing blocks that are zero filled, but with the last byte not being a zero?

I haven't tested that yet, but I share your concern. Does fio offer the ability to slap down a specific pattern like this? If not is there a logical place to extend fio to enable this?