> -----Original Message----- > From: Sitsofe Wheeler [mailto:sitsofe@xxxxxxxxx] > Sent: Thursday, December 22, 2016 2:39 AM > To: Elliott, Robert (Persistent Memory) <elliott@xxxxxxx> > Cc: fio@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 09/12] Convert group_run_stats to use bytes > instead of KiB/KB > > On 22 December 2016 at 06:08, Robert Elliott <elliott@xxxxxxx> wrote: > > In group_run_stats, convert these fields: > > * io_kb (bytes transferred) > > * agg (aggregate average bandwidth) > > * min_bw > > * max_bw > > > > from tracking either KiB or KB depending on kb_base to simply > tracking bytes. > > Rename io_kb to iobytes to match this new meaning. > > It's hopefully unlikely but have you checked this change to see that > overflow isn't provoked during arithmetic? I spent a little while > getting rid of a few cases prior to this patch by using clang's > undefined behaviour sanitizer via -fsanitize=integer and they were > more likely to occur on 32 bit platforms (which was more of a pain as > linking the sanitizer on 32 bit x86 Linux was more difficult). I think so, but I'll review the code to make sure. I've been running overnight runs on NVDIMMs which push numbers well past the 32 bit boundary: * read IOs 847TB (770TiB) * read BW 33.7GB/s (31.4GiB/s) * write IOs 419TB (381TiB) * write BW 16.7GB/s (15.6GiB/s) I did notice that the options integers (e.g., FIO_OPT_INT) are 32 bits, which prevents specifying a rate_min greater than 4 GiB/s. That looks more complicated to fix. > Another > minor point is should the variable rename have gone to io_bytes > rather > than iobytes? There's another structure (thread_data) using io_bytes, so being slightly different makes it easier to distinguish them with grep. --- Robert Elliott, HPE Persistent Memory ��.n��������+%������w��{.n�������^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
