All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Stephen  Bates" <sbates@raithlin.com>
To: "fio@vger.kernel.org" <fio@vger.kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>, David Sloan <David.Sloan@eideticom.com>
Subject: [BUG] parse.c: val_too_large incorrectly handling overly large FIO_OPT_INT values
Date: Fri, 8 May 2020 13:47:16 +0000	[thread overview]
Message-ID: <1110AE83-BA4F-46FF-B9C7-662960EC0EAD@raithlin.com> (raw)

Hi All

While trying to do some rate limiting in a high performance NVMe system we noticed that setting --rate to many values over 4G was broken (e.g. 8GB). As we dug into that it seems like the error handling for any FIO_OPTS_INT >=4GiB is not working because val_too_large in parse.c is conflating large positives uints with small negatives units uints and letting large values slide through. Tested against the tip of master (b2ed1c4a07c6 at time of writing).

I *would* submit a patch but it is actually a little tricky to keep the patch contained in val_too_large and separate the small negative number issue from the big positive number issue. So I wondered if anyone had any suggestions. I don't really want to propose something like adding a FIO_OPTS_UINT if there is an easier fix. We could also consider changing val to a signed long long but (I think) breaks the case when a ULL is passed in.

I have also created a GitHub issue here [1]. You can test with something like

static bool val_too_large(const struct fio_option *o, unsigned long long val,
                          bool is_uint)
{
        if (!o->maxval)
                return false;

        if (is_uint) {
                if ((int) val < 0)
                        return (int) val > (int) o->maxval;
                return (unsigned int) val > o->maxval;
        }

	return val > o->maxval;
}

You can test this with something like:

./fio --debug=parse --name=parse_check --rate=8G,8G --size=16k --filename /tmp/parse.dat

Cheers
 
Stephen

[1]: https://github.com/axboe/fio/issues/975
 


             reply	other threads:[~2020-05-08 13:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-08 13:47 Stephen  Bates [this message]
2020-05-08 20:48 ` [BUG] parse.c: val_too_large incorrectly handling overly large FIO_OPT_INT values Elliott, Robert (Servers)
2020-05-08 20:56   ` Stephen  Bates
2020-05-22 18:27     ` Elliott, Robert (Servers)
2020-05-22 18:59       ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1110AE83-BA4F-46FF-B9C7-662960EC0EAD@raithlin.com \
    --to=sbates@raithlin.com \
    --cc=David.Sloan@eideticom.com \
    --cc=axboe@kernel.dk \
    --cc=fio@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.