On Mon, May 11, 2020 at 01:20:32AM +0000, Damien Le Moal wrote:
> On 2020/05/06 2:57, Alexey Dobriyan wrote:
> > The following config:
> >
> > size=1%
> > io_size=1%
> >
> > will generate essentially infinite loop, because io_size is set to (2^64-1)-1.
> >
> > Parse io_size as percentage to avoid the bug.
> >
> > Note: if size% != io_size%, io_size is ignored but it is separate bug.
> >
> > Signed-off-by: Alexey Dobriyan (SK hynix) <adobriyan@xxxxxxxxx>
> > ---
> > cconv.c | 2 ++
> > options.c | 17 +++++++++++++++++
> > server.h | 2 +-
> > thread_options.h | 4 ++++
> > 4 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/cconv.c b/cconv.c
> > index 48218dc4..48b0db9e 100644
> > --- a/cconv.c
> > +++ b/cconv.c
> > @@ -102,6 +102,7 @@ void convert_thread_options_to_cpu(struct thread_options *o,
> > o->size = le64_to_cpu(top->size);
> > o->io_size = le64_to_cpu(top->io_size);
> > o->size_percent = le32_to_cpu(top->size_percent);
> > + o->io_size_percent = le32_to_cpu(top->io_size_percent);
> > o->fill_device = le32_to_cpu(top->fill_device);
> > o->file_append = le32_to_cpu(top->file_append);
> > o->file_size_low = le64_to_cpu(top->file_size_low);
> > @@ -366,6 +367,7 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
> > top->iodepth_batch_complete_max = cpu_to_le32(o->iodepth_batch_complete_max);
> > top->serialize_overlap = cpu_to_le32(o->serialize_overlap);
> > top->size_percent = cpu_to_le32(o->size_percent);
> > + top->io_size_percent = cpu_to_le32(o->io_size_percent);
> > top->fill_device = cpu_to_le32(o->fill_device);
> > top->file_append = cpu_to_le32(o->file_append);
> > top->ratecycle = cpu_to_le32(o->ratecycle);
> > diff --git a/options.c b/options.c
> > index 47b3b765..3d0c3a84 100644
> > --- a/options.c
> > +++ b/options.c
> > @@ -1475,6 +1475,22 @@ static int str_size_cb(void *data, unsigned long long *__val)
> > return 0;
> > }
> >
> > +static int str_io_size_cb(void *data, unsigned long long *__val)
> > +{
> > + struct thread_data *td = cb_data_to_td(data);
> > + unsigned long long v = *__val;
> > +
> > + if (parse_is_percent(v)) {
> > + td->o.io_size = 0;
> > + td->o.io_size_percent = -1ULL - v;
> > + dprint(FD_PARSE, "SET io_size_percent %d\n",
> > + td->o.io_size_percent);
>
> I do not see where f->io_size is set using td->o.io_size_percent. Isn't it
> needed to set it somewhere in filesetup.c ?
I recall what bugs me about this code.
It would be correct to use io_size_percent and write this:
if (f->io_size == -1ULL)
total_size = -1ULL;
else {
if (o->io_size_percent && o->io_size_percent != 100) {
uint64_t file_size;
file_size = f->io_size + f->file_offset;
f->io_size = (file_size *
o->io_size_percent) / 100;
if (f->io_size > (file_size - f->file_offset))
f->io_size = file_size - f->file_offset;
f->io_size -= (f->io_size % td_min_bs(td));
}
total_size += f->io_size;
}
f->io_size is 1M and total of 40MB worth I/O is emitted with this config
which is arguably not correct. It should be 20MB.
[global]
filename=/dev/loop0 # 1GB
direct=1
thread
rw=write
bs=4096
ioengine=psync
[j]
offset=1M
size=1M
io_size=2%
WRITE: bw=39.1MiB/s (40.0MB/s), 39.1MiB/s-39.1MiB/s (40.0MB/s-40.0MB/s), io=40.0KiB (40.0kB)
