linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: joel at joelfernandes.org (Joel Fernandes)
Subject: [PATCH v1 2/2] Add selftests for pidfd polling
Date: Fri, 26 Apr 2019 13:26:02 -0400	[thread overview]
Message-ID: <20190426172602.GD261279@google.com> (raw)
In-Reply-To: <CAKOZueuPeqnWVCv8NnQNyStWBQLybkr2qqtfQ0MjzsfSQDymzg@mail.gmail.com>

On Thu, Apr 25, 2019 at 03:07:48PM -0700, Daniel Colascione wrote:
> On Thu, Apr 25, 2019 at 2:29 PM Christian Brauner <christian at brauner.io> wrote:
> > This timing-based testing seems kinda odd to be honest. Can't we do
> > something better than this?
> 
> Agreed. Timing-based tests have a substantial risk of becoming flaky.
> We ought to be able to make these tests fully deterministic and not
> subject to breakage from odd scheduling outcomes. We don't have
> sleepable events for everything, granted, but sleep-waiting on a
> condition with exponential backoff is fine in test code. In general,
> if you start with a robust test, you can insert a sleep(100) anywhere
> and not break the logic. Violating this rule always causes pain sooner
> or later.

I prefer if you can be more specific about how to redesign the test. Please
go through the code and make suggestions there. The tests have not been flaky
in my experience. Some tests do depend on timing like the preemptoff tests,
that can't be helped. Or a performance test that calculates framedrops.

In this case, we want to make sure that the poll unblocks at the right "time"
that is when the non-leader thread exits, and not when the leader thread
exits (test 1), or when the non-leader thread exits and not when the same
non-leader previous did an execve (test 2).

These are inherently timing related. Yes it is true that if this runs in a VM
and if the VM CPU is preempted for a couple seconds, then the test can fail
falsely. Still I would argue such a failure scenario of a multi-second CPU
lock-up can cause more serious issues like RCU stalls, and that's not a test
issue. We can increase the sleep intervals if you want, to reduce the risk of
such scenarios.

I would love to make the test not depend on timing, but I don't know how. And
the tests caught issues that I had in my development flow, so the tests
worked quite well in my experience.

thanks,

 - Joel

WARNING: multiple messages have this Message-ID (diff)
From: joel@joelfernandes.org (Joel Fernandes)
Subject: [PATCH v1 2/2] Add selftests for pidfd polling
Date: Fri, 26 Apr 2019 13:26:02 -0400	[thread overview]
Message-ID: <20190426172602.GD261279@google.com> (raw)
Message-ID: <20190426172602.dNwcGZJPxwzulAU4Z6314bk6WgnXxrgZxNXWPVJ80DM@z> (raw)
In-Reply-To: <CAKOZueuPeqnWVCv8NnQNyStWBQLybkr2qqtfQ0MjzsfSQDymzg@mail.gmail.com>

On Thu, Apr 25, 2019@03:07:48PM -0700, Daniel Colascione wrote:
> On Thu, Apr 25, 2019@2:29 PM Christian Brauner <christian@brauner.io> wrote:
> > This timing-based testing seems kinda odd to be honest. Can't we do
> > something better than this?
> 
> Agreed. Timing-based tests have a substantial risk of becoming flaky.
> We ought to be able to make these tests fully deterministic and not
> subject to breakage from odd scheduling outcomes. We don't have
> sleepable events for everything, granted, but sleep-waiting on a
> condition with exponential backoff is fine in test code. In general,
> if you start with a robust test, you can insert a sleep(100) anywhere
> and not break the logic. Violating this rule always causes pain sooner
> or later.

I prefer if you can be more specific about how to redesign the test. Please
go through the code and make suggestions there. The tests have not been flaky
in my experience. Some tests do depend on timing like the preemptoff tests,
that can't be helped. Or a performance test that calculates framedrops.

In this case, we want to make sure that the poll unblocks at the right "time"
that is when the non-leader thread exits, and not when the leader thread
exits (test 1), or when the non-leader thread exits and not when the same
non-leader previous did an execve (test 2).

These are inherently timing related. Yes it is true that if this runs in a VM
and if the VM CPU is preempted for a couple seconds, then the test can fail
falsely. Still I would argue such a failure scenario of a multi-second CPU
lock-up can cause more serious issues like RCU stalls, and that's not a test
issue. We can increase the sleep intervals if you want, to reduce the risk of
such scenarios.

I would love to make the test not depend on timing, but I don't know how. And
the tests caught issues that I had in my development flow, so the tests
worked quite well in my experience.

thanks,

 - Joel

  parent reply	other threads:[~2019-04-26 17:26 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-25 19:00 [PATCH v1 1/2] Add polling support to pidfd joel
2019-04-25 19:00 ` Joel Fernandes (Google)
2019-04-25 19:00 ` [PATCH v1 2/2] Add selftests for pidfd polling joel
2019-04-25 19:00   ` Joel Fernandes (Google)
2019-04-25 20:00   ` tycho
2019-04-25 20:00     ` Tycho Andersen
2019-04-26 13:47     ` joel
2019-04-26 13:47       ` Joel Fernandes
2019-04-25 21:29   ` christian
2019-04-25 21:29     ` Christian Brauner
2019-04-25 22:07     ` dancol
2019-04-25 22:07       ` Daniel Colascione
2019-04-26 17:26       ` joel [this message]
2019-04-26 17:26         ` Joel Fernandes
2019-04-26 19:35         ` dancol
2019-04-26 19:35           ` Daniel Colascione
2019-04-26 20:31           ` joel
2019-04-26 20:31             ` Joel Fernandes
2019-04-26 13:42     ` joel
2019-04-26 13:42       ` Joel Fernandes
2019-04-25 22:24 ` [PATCH v1 1/2] Add polling support to pidfd christian
2019-04-25 22:24   ` Christian Brauner
2019-04-26 14:23   ` joel
2019-04-26 14:23     ` Joel Fernandes
2019-04-26 15:21     ` christian
2019-04-26 15:21       ` Christian Brauner
2019-04-26 15:31       ` christian
2019-04-26 15:31         ` Christian Brauner
2019-04-28 16:24   ` oleg
2019-04-28 16:24     ` Oleg Nesterov
2019-04-29 14:02     ` joel
2019-04-29 14:02       ` Joel Fernandes
2019-04-29 14:07       ` joel
2019-04-29 14:07         ` Joel Fernandes
2019-04-29 14:25         ` oleg
2019-04-29 14:25           ` Oleg Nesterov
2019-04-29 14:20       ` oleg
2019-04-29 14:20         ` Oleg Nesterov
2019-04-29 16:32         ` joel
2019-04-29 16:32           ` Joel Fernandes
2019-04-30 11:53           ` oleg
2019-04-30 11:53             ` Oleg Nesterov
2019-04-30 12:07             ` oleg
2019-04-30 12:07               ` Oleg Nesterov
2019-04-30 15:49             ` joel
2019-04-30 15:49               ` Joel Fernandes
2019-04-26 14:58 ` christian
2019-04-26 14:58   ` Christian Brauner

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=20190426172602.GD261279@google.com \
    --to=linux-kselftest@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).