5:50 a.m.
On Wed, Jan 02, 2019 at 09:39:10PM -0600, Eric Blake wrote:
> +The default thresholds are high 95% and low 80%. The
thresholds are
> +expressed as percentages of C<cache-max-size>, and may be greater than
> +100.
Do the thresholds support fractional percentages, like 90.5, or must
they be integral? Should we enforce that low threshold < high
threshold? What does it really mean to have a threshold > 100 - that we
can never reach the high threshold?
We don't support fractional percentages, but we could do, although it
seems like a overengineering (what's the real difference between a
threshold of 50% and 50.1% ?!). We do enforce low < high. A
percentage > 100% just means that we have a higher threshold (eg.
low = 160%, high = 190% behaves the same if cache-max-size is halved).
In the next version I have clarified and simplified the wording in the
manual.
> +/* Do we support reclaiming cache blocks? */
> +#ifdef FALLOC_FL_PUNCH_HOLE
> +#define HAVE_CACHE_RECLAIM 1
> +#else
> +#undef HAVE_CACHE_RECLAIM
> +#endif
> +
Should the man page mention that max cache size can only be enforced
with kernel support? Do we want to go further and probe whether
FALLOC_FL_PUNCH_HOLE works on $TMPDIR? (Even if the kernel supports
punching holes, there are some filesystems that don't).
I'll add it to the manual, but it seems irksome to actually test it at
run time ...
> + cache_allocated = statbuf.st_blocks * UINT64_C(512);
Is that calculation always going to be accurate, even when more
disks/file systems move towards 4k minimum access?
My reading of stat(2) is st_blocks is always expressed in 512 byte
blocks, independent of the sector size.
> + nbdkit_debug ("cache: reclaiming block %" PRIu64,
reclaim_blk);
Where does this prioritize clean blocks over dirty blocks? Or is the
.pod change inaccurate given later changes to implementation?
Right, it doesn't. I'll fix the manual.
> +#ifdef FALLOC_FL_PUNCH_HOLE
> + if (fallocate (fd, FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE,
> + reclaim_blk * BLKSIZE, BLKSIZE) == -1) {
> + nbdkit_error ("cache: reclaiming cache blocks: "
> + "fallocate: FALLOC_FL_PUNCH_HOLE: %m");
> + return;
> + }
> +#else
> +#error "no implementation for punching holes"
This #error should be unreachable, given the definition of
HAVE_CACHE_RECLAIM based on FALLOC_FL_PUNCH_HOLE.
So I left this in, in case we change HAVE_CACHE_RECLAIM for
implementing hole punching on another platform but forget to update
this code.
>> +static void
>> +reclaim_any (void)
>> +{
>> + /* Find the next block in the cache. */
>> + reclaim_blk = bitmap_next (&bm, reclaim_blk+1);
>> + if (reclaim_blk == -1) /* wrap around */
>> + reclaim_blk = bitmap_next (&bm, 0);
>> +
>> + reclaim_block ();
Even when reclaiming any, should we try to prioritize blocks that are
found in the LRU cache's bm[1] but not bm[0], as we do have at least
that bit of knowledge about LRU patterns? Otherwise, this can end up
claiming the most recently used block, if that happens to be the next
block in our cycling through the entire cache. Or is that too much
layering violation, where it would unnecessarily tie this code to the
current lru implementation?
I think we'd end up needing 3 states. I'll add a to-do comment ...
Thanks,
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org