public inbox for [email protected]
 help / color / mirror / Atom feed
From: Ammar Faizi <[email protected]>
To: Jens Axboe <[email protected]>
Cc: Alviro Iskandar Setiawan <[email protected]>,
	Niklas Cassel <[email protected]>,
	 Vincent Fu <[email protected]>,
	fio Mailing List <[email protected]>,
	 "GNU/Weeb Mailing List" <[email protected]>
Subject: Re: [PATCH v1 0/8] fio error handling fixes
Date: Sat, 30 Apr 2022 03:15:07 +0700	[thread overview]
Message-ID: <CAFBCWQKun4kpe-tHKsTUpERwh0zxeUfn1=b6yNU-aRYt8Fdfgg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>

On Sat, Apr 30, 2022 at 1:21 AM Jens Axboe <[email protected]> wrote:
> On 4/28/22 6:46 PM, Ammar Faizi wrote:
> > From: Ammar Faizi <[email protected]>
> >
> > Hi Jens,
> >
> > This series contains patches that were dropped from my previous thread:
> >
> >   https://lore.kernel.org/fio/[email protected]
> >
> > Plus, I have more patches in this series. This series contains error
> > handling fixes, mostly about ENOMEM. There are 8 patches in this
> > series. All of them are ENOMEM handling stuff except that patch #4 has
> > extra fixes and a bit of refactoring.
>
> While I agree on good error handling in general, I'm a bit dubious at
> malloc() related NULL returns. Have you ever seen malloc() return NULL?
> On Linux, or somewhere else? Most systems overcommit themselves to
> death, and things will death spiral before you ever get NULL.

Yes, you're right. malloc() returns NULL is a very rare case. I don't
think fio users will ever see that in general.

I have seen malloc() return NULL when a program is in jail (e.g.
inside cgroup, or whatever that puts a limit on the memory resource).
But obviously, it is not reasonable for fio. Although that's one of my
motivations, it's actually not the main one that brought me to do this
series.

> I'm happy to be convinced otherwise, just naturally skeptical that
> there's any real value in making changes like this.

I am a bit bothered seeing inconsistent error handling in the
codebase. From what I see in the fio codebase today, sometimes we
handle ENOMEM cases, sometimes we don't. I want to make it consistent
in the hope we can improve the quality of the overall error handling.

If you are fine with that, I can send more incremental patches for
review in the future to fix those inconsistencies. It may be tiring
and progressive work. But I am okay doing that.

However, if you NAK this series. I will start fixing other broken
things I can find, like in patch #4 for example. And that's fine.

-- 
Ammar Faizi

  reply	other threads:[~2022-04-29 20:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-29  0:46 [PATCH v1 0/8] fio error handling fixes Ammar Faizi
2022-04-29  0:46 ` [PATCH v1 1/8] cgroup: Add ENOMEM handling on a `malloc()` call Ammar Faizi
2022-04-29 18:20   ` Vincent Fu
2022-04-30  3:25   ` Alviro Iskandar Setiawan
2022-04-29  0:46 ` [PATCH v1 2/8] stat: Add ENOMEM handling on `malloc()` / `calloc()` calls Ammar Faizi
2022-04-29  0:47 ` [PATCH v1 3/8] engines/net: Add ENOMEM handling on a `malloc()` call Ammar Faizi
2022-04-29 18:20   ` Vincent Fu
2022-04-29  0:47 ` [PATCH v1 4/8] blktrace: Fix broken error handling in `merge_blktrace_iologs()` Ammar Faizi
2022-04-29  0:47 ` [PATCH v1 5/8] blktrace: Add ENOMEM handling when allocating @ipo Ammar Faizi
2022-04-29  0:47 ` [PATCH v1 6/8] blktrace: Add ENOMEM handling in `trace_add_open_close_event()` and its callers Ammar Faizi
2022-04-29  0:47 ` [PATCH v1 7/8] client: Add ENOMEM handling on `realloc()` calls Ammar Faizi
2022-04-29  0:47 ` [PATCH v1 8/8] client: Add ENOMEM handling on `malloc()`, `calloc()` and `strdup()` calls Ammar Faizi
2022-04-30 13:08   ` Jens Axboe
2022-04-29 18:21 ` [PATCH v1 0/8] fio error handling fixes Jens Axboe
2022-04-29 20:15   ` Ammar Faizi [this message]
2022-04-29 20:37     ` 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='CAFBCWQKun4kpe-tHKsTUpERwh0zxeUfn1=b6yNU-aRYt8Fdfgg@mail.gmail.com' \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox