Page MenuHomeFreeBSD

loader bcache: Track unconsumed readahead
ClosedPublic

Authored by cperciva on Oct 1 2021, 5:14 AM.

Details

Summary

The loader bcache attempts to determine whether readahead is
useful, increasing or decreasing its readahead length based on
whether a read could be serviced out of the cache. This resulted
in two unfortunate behaviours:

  1. A series of consecutive 32 kB reads are requested and bcache

performs 16 kB readaheads. For each read, bcache determines
that, since only the first 16 kB is already in the cache, the
readahead was not useful, and keeps the readahead at the
minimum (16 kB) level.

  1. A series of consecutive 32 kB reads are requested and bcache

starts with a 32 kB readahead resulting in a 64 kB being read on
the first request. The second 32 kB request can be serviced out
of the cache, and bcache responds by doubling its readahead
length to 64 kB. The third 32 kB request cannot be serviced out
of the cache, and bcache reduces its readahead length back down
to 32 kB.

The first syndrome converts a series of 32 kB reads into a series
of (misaligned) 32 kB reads, while the second syndrome converts
a series of 32 kB reads into a series of 64 kB reads; in both cases
we do not increase the readahead length to its limit (currently
128 kB) no matter how many consecutive read requests are made.

This change avoids this problem by tracking the "unconsumed
readahead" length; readahead is deemed to be useful (and the
read-ahead length is potentially increased) not only if a request
was completely serviced out of the cache, but also if *any* of the
request was serviced out of the cache and that length matches
the amount of unconsumed readahead. Conversely, we now only
reduce the readahead length in cases where there was unconsumed
readahead data.

In my testing on an EC2 c5.xlarge instance, this change reduces the
boot time by roughly 120 ms.

Test Plan

This patch applies on top of D32249.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cperciva added reviewers: imp, tsoome.

+allanjude since I think he had some systems with network booting which might benefit from this.

This revision is now accepted and ready to land.Oct 1 2021, 7:42 AM
stand/common/bcache.c
248

It would be good to document what 'if approrpiate' means here. The kernel has extensive comments explaining what's going on and that would be useful here.

253

This line is likely too wide now

stand/common/bcache.c
248

Will do.

253

This file isn't consistently styled, but yes I'll rewrap that bit.

This revision was automatically updated to reflect the committed changes.