Hi Jens,
As I was verifying the latest code from head, I realized that we
actually did not need the IO_U_F_FILLED flag, we can just
use(overload) the buf_filled_len to check if the buffer is actually
filled (as this is reset to 0 on a read). This also gave me slight
performance bump. Attached is the patch for it.
thanks
-radha
On Tue, Jul 13, 2010 at 11:44 PM, Jens Axboe <jaxboe@xxxxxxxxxxxx> wrote:
> On 07/14/2010 08:33 AM, Jens Axboe wrote:
>> On 07/14/2010 02:13 AM, Radha Ramachandran wrote:
>>> Hi Jens,
>>> I made changes to fio so we wld re-use the already populated io_u
>>> buffer (when there is a non-random pattern) during writes. That way
>>> only the header will be re-calculated for every I/O. This way the
>>> buffer wld get populated in the beginning and as long as the
>>> subsequent ios using the same io_u structure are writes and have same
>>> or less block size, it wld get re-used. If any of the subsequent i/o
>>> is a read or has a block size greater than the pre-filled one, then
>>> the buffer is invalidated and will be re-filled at the next write.
>>>
>>> Reason for this risky change: (Performance)
>>> I tested this change on a tmpfs(with no swap backing), with the
>>> following config file:
>>> [sscan_write]
>>> filename=/mytmpfs/datafile.tmp
>>> rw=write
>>> bs=64k
>>> size=3G
>>> ioengine=libaio
>>> iodepth=1024
>>> iodepth_low=512
>>> runtime=10800
>>> bwavgtime=5000
>>> thread=1
>>> do_verify=0
>>> verify=meta
>>> verify_pattern=0x55aaa55a
>>> verify_interval=4k
>>> continue_on_error=1
>>>
>>> fio-1-41-6 gave 306MB/s and the new change had a performance of 1546MB/s
>>>
>>> Side effects/Risks:
>>> There is a risk with this fix, that if the buffer gets corrupted then
>>> the subsequent writes will also be corrupt. I think for both
>>> sequential writes and random writes (with verify, where the I/O log is
>>> replayed) we shld be able to find the first I/O that started with the
>>> corruption and if the buffer is getting corrupted, there are other
>>> issues here.
>>>
>>> Testing:
>>> I have tested this fix with sequential write(verify)/random read write
>>> mix combination(with verify).
>>>
>>> I think I have taken care of most of the case, but please let me know
>>> if there is anything I have missed. I have attached the patch along
>>> with this email. I think the performance improvement outweighs the
>>> risk associated with the fix. But I will let you decide if you wld
>>> like to pick it up.
>>
>> I will pick this up, the fill time is the reason for some of the
>> other hoops we jump through to try and avoid that when possible.
>> I don't think the risk of memory corruption is something we need
>> to consider. That could just as easily happen after the data
>> has been read anyway, both cases would result in a verify error.
>>
>
> I made a small change to turn ->buf_filled into an io_u
> flag. It would be nice if you could retest the current
> -git state and ensure that it works properly. I'm on the
> road the next ~10 days, so wont have a lot of testing time
> on my hands.
>
> --
> Jens Axboe
>
>
diff --git a/io_u.c b/io_u.c
index 88b2b9f..f27cdda 100644
--- a/io_u.c
+++ b/io_u.c
@@ -988,7 +988,6 @@ struct io_u *get_io_u(struct thread_data *td)
* Reset the buf_filled parameters so next time if the
* buffer is used for writes it is refilled.
*/
- io_u->flags &= ~IO_U_F_FILLED;
io_u->buf_filled_len = 0;
}
}
diff --git a/ioengine.h b/ioengine.h
index 343b06f..e9f5d92 100644
--- a/ioengine.h
+++ b/ioengine.h
@@ -8,7 +8,6 @@ enum {
IO_U_F_FLIGHT = 1 << 1,
IO_U_F_FREE_DEF = 1 << 2,
IO_U_F_IN_CUR_DEPTH = 1 << 3,
- IO_U_F_FILLED = 1 << 4,
};
/*
diff --git a/verify.c b/verify.c
index 098c19b..42ea462 100644
--- a/verify.c
+++ b/verify.c
@@ -30,22 +30,19 @@ void fill_pattern(struct thread_data *td, void *p, unsigned int len, struct io_u
fill_random_buf(p, len);
break;
case 1:
- if ((io_u->flags & IO_U_F_FILLED) &&
- io_u->buf_filled_len >= len) {
+ if (io_u->buf_filled_len >= len) {
dprint(FD_VERIFY, "using already filled verify pattern b=0 len=%u\n", len);
return;
}
dprint(FD_VERIFY, "fill verify pattern b=0 len=%u\n", len);
memset(p, td->o.verify_pattern[0], len);
- io_u->flags |= IO_U_F_FILLED;
io_u->buf_filled_len = len;
break;
default: {
unsigned int i = 0, size = 0;
unsigned char *b = p;
- if ((io_u->flags & IO_U_F_FILLED) &&
- io_u->buf_filled_len >= len) {
+ if (io_u->buf_filled_len >= len) {
dprint(FD_VERIFY, "using already filled verify pattern b=%d len=%u\n",
td->o.verify_pattern_bytes, len);
return;
@@ -60,7 +57,6 @@ void fill_pattern(struct thread_data *td, void *p, unsigned int len, struct io_u
memcpy(b+i, td->o.verify_pattern, size);
i += size;
}
- io_u->flags |= IO_U_F_FILLED;
io_u->buf_filled_len = len;
break;
}
