Page MenuHomeFreeBSD

devd tests: Fix client_test
Needs ReviewPublic

Authored by markj on Fri, Nov 15, 5:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 2:06 AM
Unknown Object (File)
Mon, Nov 18, 3:23 AM
Unknown Object (File)
Mon, Nov 18, 12:22 AM
Unknown Object (File)
Sun, Nov 17, 2:50 PM
Unknown Object (File)
Sun, Nov 17, 2:20 PM
Subscribers

Details

Reviewers
asomers
Summary

The loop doesn't check for overflow of the event buffer, which can
easily happen if other tests are running in parallel (the bectl tests in
particular trigger devd events).

When that overflow occurs, a funny thing can happen: the loop ends up
trying to read 0 bytes from the socket, succeeds, and then prints its
buffer to stdout. It does this as fast as possible, eventually timing
out. Then, because kyua wants to log the test's output, it slurps the
output file into memory so that it can insert it into the test db. This
output file is quite large, usually around 8GB when I see it happen, and
is large enough to trigger an OOM kill in my test suite runner VM.

Fix the test: use a larger buffer and fail the test if we fill it before
both events are observed. Also don't print the output buffer on every
loop iteration, since unlike the seqpacket test that will just print the
same output over and over.

Sponsored by: Klara, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 60617
Build 57501: arc lint + arc unit

Event Timeline

markj requested review of this revision.Fri, Nov 15, 5:57 PM

So read only ever returns one record
and we know the each line is at max 1024 - sizeof(void *)
so why do we need to keep the whole thing?
devctl dev_read() doesn't split the records.

In D47625#1085688, @imp wrote:

So read only ever returns one record

Are you sure that's true? This isn't reading from /dev/devctl directly. It's reading from /var/run/devd.pipe, which is a stream socket.

and we know the each line is at max 1024 - sizeof(void *)
so why do we need to keep the whole thing?
devctl dev_read() doesn't split the records.

In D47625#1085688, @imp wrote:

So read only ever returns one record

Are you sure that's true? This isn't reading from /dev/devctl directly. It's reading from /var/run/devd.pipe, which is a stream socket.

Oh! That's the whole bug right there. Nothing to do with the 'bursts' per-se, but it's all about that. When I wrote this, I thought
of the devd interface and wanted something that would test *THAT*, but this doesn't have insight enough to do that. Of course
based on the comments, I clearly only half thought this...

Couple of questions / nits now that that is out of the way.

and we know the each line is at max 1024 - sizeof(void *)
so why do we need to keep the whole thing?
devctl dev_read() doesn't split the records.

sbin/devd/tests/client_test.c
144

1MB is tens of thousands of entries typically, so we should never overflow.... Might want to note that...

151

I'd add "We have to read from the streaming interface of /var/run/devd.pipe, so can't count on record boundaries." to act as a big clue for why we have to do this.

159

Shouldn't this be strictly > ?