I think I've made three fixes over the original two commits.
1. non O_DIRECT dio support -> committed -> now reverted
2. segfault on fio_memfree() -> committed -> now reverted
3. this one
As far as I've seen this is it.
(I think pre read can do the same as this function, to avoid
irrelevant errors, though pre read on dio is contradictory)
2017-09-05 17:39 GMT+03:00 Jens Axboe <axboe@xxxxxxxxx>:
> On 09/05/2017 12:14 AM, Tomohiro Kusumi wrote:
>> 2017-09-05 8:59 GMT+03:00 Sitsofe Wheeler <sitsofe@xxxxxxxxx>:
>>> On 4 September 2017 at 20:48, Tomohiro Kusumi <kusumi.tomohiro@xxxxxxxxx> wrote:
>>>> 2017-09-04 22:36 GMT+03:00 Sitsofe Wheeler <sitsofe@xxxxxxxxx>:
>>>>> Hi,
>>>>> On 4 September 2017 at 16:23, <kusumi.tomohiro@xxxxxxxxx> wrote:
>>>>>> From: Tomohiro Kusumi <tkusumi@xxxxxxxxxx>
>>>>>>
>>>>>> 8c43ba62('filesetup: align layout buffer') and
>>>>>> 6e344dc3('filesetup: keep OS_O_DIRECT flag when pre-allocating file')
>>>>>> need to keep the valid transfer size throughout the entire writes.
>>>>>>
>>>>>> The write(2) size may be truncated on the last write and break the
>>>>>> dio requirement. This results in td_verror() in the output.
>>>>>>
>>> [...]
>>>>>> ---
>>>>>> filesetup.c | 13 ++++++++++---
>>>>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/filesetup.c b/filesetup.c
>>>>>> index 5e8ea35..42d95db 100644
>>>>>> --- a/filesetup.c
>>>>>> +++ b/filesetup.c
>>>>>> @@ -112,6 +112,7 @@ static int extend_file(struct thread_data *td, struct fio_file *f)
>>>>>> unsigned long long left;
>>>>>> unsigned int bs, alloc_size = 0;
>>>>>> char *b = NULL;
>>>>>> + bool done = false;
>>>>>>
>>>>>> if (read_only) {
>>>>>> log_err("fio: refusing extend of file due to read-only\n");
>>>>>> @@ -211,11 +212,15 @@ static int extend_file(struct thread_data *td, struct fio_file *f)
>>>>>> goto err;
>>>>>> }
>>>>>>
>>>>>> - while (left && !td->terminate) {
>>>>>> + while (!done && !td->terminate) {
>>>>>> ssize_t r;
>>>>>>
>>>>>> - if (bs > left)
>>>>>> - bs = left;
>>>>>> + /* If bs >= left this is the last write */
>>>>>> + if (bs > left) {
>>>>>> + done = true;
>>>>>> + if (!td->o.odirect)
>>>>>> + bs = left;
>>>>>> + }
>>>>>>
>>>>>> fill_io_buffer(td, b, bs, bs);
>>>>>>
>>>>>
>>>>> Hmm. Won't this result in a file that is bigger than requested when
>>>>> direct=1 and the maximum bs isn't an exact multiple of the extended
>>>>> filesize? Should it start trying the minimum block size at this stage
>>>>> with direct=1? If that goes on to not fit wouldn't it be better to do
>>>>> less given that the main fio job won't be able to fill the gap either?
>>>>
>>>> The td's are also under the dio requirements.
>>>> If you really want the exact size, you can truncate(2) it.
>>>
>>> Technically didn't fio already truncate the file to the correct size
>>> prior to starting direct layout over on
>>> https://github.com/axboe/fio/blob/d33db728d79386d544be93c24f4e3383f2a47143/filesetup.c#L191
>>> ?
>>
>> You were talking about the size *after* the write....
>
> How many fixups for a simple "use O_DIRECT for layout if the job file
> has direct=1 set" have we done now? I'm going to revert the whole thing,
> and if someone can convince me that a simple clean patch that gets all
> of it right exists, then please do send it. But until that happens,
> we're going back to the old behavior of just using buffered IO to lay it
> out.
>
> --
> Jens Axboe
>
--
To unsubscribe from this list: send the line "unsubscribe fio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
