Re: Possible bug when setting compression

[Jens Axboe <axboe@kernel.dk>]

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

On 03/14/2014 02:09 PM, Jens Axboe wrote:
> On 03/14/2014 01:45 PM, Matthew Eaton wrote:
>>> Interesting, it would make sense to simply not scramble if the
>>> compression
>>> settings are set. That is what happens with refill_buffers right now,
>>> it has
>>> precedence over the scrambling. Or if you give a buffer pattern, that
>>> also
>>> disables the scrambling. I'll add the same for the compression settings.
>>>
>>> http://git.kernel.dk/?p=fio.git;a=commit;h=0574c885b82aea0332ab5fa35af84db0f3946726
>>>
>>>
>>> Care to give it a test spin, just to be on the safe side?
>>>
>>> --
>>> Jens Axboe
>>>
>>
>> Hey Jens,
>>
>> According to my test results below it worked for
>> buffer_compress_percentage, but not zero_buffers.  Also, it looks like
>> buffer_compress_percentage=0 is broken, unless =0 means disabling that
>> option altogether.
>>
>> fio-2.1.6.1-11-g0574
>>
>> buffer_compress_percentage=100
>> iops=83436
>>
>> buffer_compress_percentage=50
>> iops=78625
>>
>> buffer_compress_percentage=1
>> iops=52751
>>
>> buffer_compress_percentage=0
>> iops=69433
>>
>> zero_buffers
>> iops=69470
>>
>> scramble_buffers=0
>> zero_buffers
>> iops=83369
>>
>
> Doh yes, we can't use that as an enabled flag. Can you apply this on top
> of current git and see if it works? Untested...

So that wont work as-is, since the parser will bypass the store when the 
callback is specified. It needs a one-liner, please try this updated one 
instead.

-- 
Jens Axboe


[-- Attachment #2: fio-compress-v2.patch --]
[-- Type: text/x-patch, Size: 2394 bytes --]

diff --git a/fio.h b/fio.h
index 52f1def7a28d..befdce31ee20 100644
--- a/fio.h
+++ b/fio.h
@@ -71,6 +71,7 @@ enum {
 	TD_F_SCRAMBLE_BUFFERS	= 16,
 	TD_F_VER_NONE		= 32,
 	TD_F_PROFILE_OPS	= 64,
+	TD_F_COMPRESS		= 128,
 };
 
 enum {
diff --git a/init.c b/init.c
index 0a872c0f1eed..73ec9eb22a8d 100644
--- a/init.c
+++ b/init.c
@@ -856,11 +856,6 @@ int ioengine_load(struct thread_data *td)
 	return 0;
 }
 
-static int compression_enabled(struct thread_options *o)
-{
-	return o->compress_percentage || o->compress_chunk;
-}
-
 static void init_flags(struct thread_data *td)
 {
 	struct thread_options *o = &td->o;
@@ -873,14 +868,8 @@ static void init_flags(struct thread_data *td)
 		td->flags |= TD_F_READ_IOLOG;
 	if (o->refill_buffers)
 		td->flags |= TD_F_REFILL_BUFFERS;
-
-	/*
-	 * Don't scramble buffers if we set any of the compression
-	 * settings
-	 */
-	if (o->scramble_buffers && !compression_enabled(o))
+	if (o->scramble_buffers)
 		td->flags |= TD_F_SCRAMBLE_BUFFERS;
-
 	if (o->verify != VERIFY_NONE)
 		td->flags |= TD_F_VER_NONE;
 }
diff --git a/io_u.c b/io_u.c
index 0b86d9f3c281..14645aeed50c 100644
--- a/io_u.c
+++ b/io_u.c
@@ -1490,7 +1490,8 @@ struct io_u *get_io_u(struct thread_data *td)
 			if (td->flags & TD_F_REFILL_BUFFERS) {
 				io_u_fill_buffer(td, io_u,
 					io_u->xfer_buflen, io_u->xfer_buflen);
-			} else if (td->flags & TD_F_SCRAMBLE_BUFFERS)
+			} else if ((td->flags & TD_F_SCRAMBLE_BUFFERS) &&
+				   !(td->flags & TD_F_COMPRESS))
 				do_scramble = 1;
 			if (td->flags & TD_F_VER_NONE) {
 				populate_verify_io_u(td, io_u);
diff --git a/options.c b/options.c
index 4ff4c9b42a7b..1ffb4b177d25 100644
--- a/options.c
+++ b/options.c
@@ -1002,6 +1002,15 @@ static int str_buffer_pattern_cb(void *data, const char *input)
 	return ret;
 }
 
+static int str_buffer_compress_cb(void *data, unsigned long long *il)
+{
+	struct thread_data *td = data;
+
+	td->flags |= TD_F_COMPRESS;
+	td->o.compress_percentage = *il;
+	return 0;
+}
+
 static int str_verify_pattern_cb(void *data, const char *input)
 {
 	struct thread_data *td = data;
@@ -3119,6 +3128,7 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 		.lname	= "Buffer compression percentage",
 		.type	= FIO_OPT_INT,
 		.off1	= td_var_offset(compress_percentage),
+		.cb	= str_buffer_compress_cb,
 		.maxval	= 100,
 		.minval	= 0,
 		.help	= "How compressible the buffer is (approximately)",