Adopted 0/-1 as none/all hints in bd layer

This matches other functions where we may accept unbounded ranges, e.g.,
lfsr_rbyd_appendattrs, lfsr_data_slice, etc.

The motivation is that these constants, all zeros and all ones, often
have special encodings in ISAs due to their commonality. That and
constants are cheaper than runtime-dependent values such as block_size.
(block_size may be a compile-time constant at some point, but we will
still need to support runtime-determined block_sizes)

I thought this would be a quick change, but it led to an interesting
overflow condition in lfsr_bd_read when we calculate the cache
alignment/limit.

Fortunately, the rewritten expression is quite a bit cleaner.

The expression rewrite did drown out any code cost benefit, but I'm
keeping this change because it makes the code a bit more readable/
writeable when there's a simple "unbounded" value:

           code          stack
  before: 33572           2800
  after:  33584 (+0.0%)   2792 (-0.3%)
This commit is contained in:
Christopher Haster
2024-02-19 15:24:53 -06:00
parent 769f761a8b
commit 543fb976b4
+15 -16
View File
@@ -178,14 +178,14 @@ static int lfsr_bd_read(lfs_t *lfs,
// load to cache, first condition can no longer fail // load to cache, first condition can no longer fail
lfs_size_t off__ = lfs_aligndown(off_, lfs->cfg->read_size); lfs_size_t off__ = lfs_aligndown(off_, lfs->cfg->read_size);
lfs_size_t size__ = lfs_min( // watch out for overflow when hint_=-1
lfs_min( lfs_size_t size__ = lfs_alignup(
lfs_alignup( (off_-off__) + lfs_min(
off_+lfs_max(size_, hint_), lfs_max(size_, hint_),
lfs->cfg->read_size), lfs_min(
lfs->cfg->block_size) lfs->cfg->cache_size - (off_-off__),
- off__, lfs->cfg->block_size - off_)),
lfs->cfg->cache_size); lfs->cfg->read_size);
int err = lfsr_bd_read_(lfs, block, off__, int err = lfsr_bd_read_(lfs, block, off__,
lfs->rcache.buffer, size__); lfs->rcache.buffer, size__);
if (err) { if (err) {
@@ -2088,8 +2088,8 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// checksum the revision count to get the cksum started // checksum the revision count to get the cksum started
uint32_t cksum = 0; uint32_t cksum = 0;
int err = lfsr_bd_cksum(lfs, block, 0, lfs->cfg->block_size, int err = lfsr_bd_cksum(lfs, block, 0, -1, sizeof(uint32_t),
sizeof(uint32_t), &cksum); &cksum);
if (err) { if (err) {
return err; return err;
} }
@@ -2115,8 +2115,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
lfsr_tag_t tag; lfsr_tag_t tag;
lfsr_rid_t weight__; lfsr_rid_t weight__;
lfs_size_t size; lfs_size_t size;
lfs_ssize_t d = lfsr_bd_readtag(lfs, lfs_ssize_t d = lfsr_bd_readtag(lfs, block, off, -1,
block, off, lfs->cfg->block_size,
&tag, &weight__, &size, &cksum); &tag, &weight__, &size, &cksum);
if (d < 0) { if (d < 0) {
if (d == LFS_ERR_INVAL || d == LFS_ERR_CORRUPT) { if (d == LFS_ERR_INVAL || d == LFS_ERR_CORRUPT) {
@@ -2139,7 +2138,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// not an end-of-commit cksum // not an end-of-commit cksum
if (!lfsr_tag_isalt(tag) && lfsr_tag_suptype(tag) != LFSR_TAG_CKSUM) { if (!lfsr_tag_isalt(tag) && lfsr_tag_suptype(tag) != LFSR_TAG_CKSUM) {
// cksum the entry, hopefully leaving it in the cache // cksum the entry, hopefully leaving it in the cache
err = lfsr_bd_cksum(lfs, block, off_, lfs->cfg->block_size, size, err = lfsr_bd_cksum(lfs, block, off_, -1, size,
&cksum); &cksum);
if (err) { if (err) {
if (err == LFS_ERR_CORRUPT) { if (err == LFS_ERR_CORRUPT) {
@@ -2168,7 +2167,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// is an end-of-commit cksum // is an end-of-commit cksum
} else if (!lfsr_tag_isalt(tag)) { } else if (!lfsr_tag_isalt(tag)) {
uint32_t cksum_ = 0; uint32_t cksum_ = 0;
err = lfsr_bd_read(lfs, block, off_, lfs->cfg->block_size, err = lfsr_bd_read(lfs, block, off_, -1,
&cksum_, sizeof(uint32_t)); &cksum_, sizeof(uint32_t));
if (err) { if (err) {
if (err == LFS_ERR_CORRUPT) { if (err == LFS_ERR_CORRUPT) {
@@ -5534,7 +5533,7 @@ static int lfsr_mdir_alloc__(lfs_t *lfs, lfsr_mdir_t *mdir, lfsr_smid_t mid) {
// we use whatever is on-disk to avoid needing to rewrite the // we use whatever is on-disk to avoid needing to rewrite the
// redund block // redund block
uint32_t rev; uint32_t rev;
int err = lfsr_bd_read(lfs, mdir->rbyd.blocks[1], 0, sizeof(uint32_t), int err = lfsr_bd_read(lfs, mdir->rbyd.blocks[1], 0, 0,
&rev, sizeof(uint32_t)); &rev, sizeof(uint32_t));
if (err && err != LFS_ERR_CORRUPT) { if (err && err != LFS_ERR_CORRUPT) {
return err; return err;
@@ -5571,7 +5570,7 @@ static int lfsr_mdir_swap__(lfs_t *lfs, lfsr_mdir_t *mdir_,
// first thing we need to do is read our current revision count // first thing we need to do is read our current revision count
uint32_t rev; uint32_t rev;
int err = lfsr_bd_read(lfs, mdir->rbyd.blocks[0], 0, sizeof(uint32_t), int err = lfsr_bd_read(lfs, mdir->rbyd.blocks[0], 0, 0,
&rev, sizeof(uint32_t)); &rev, sizeof(uint32_t));
if (err && err != LFS_ERR_CORRUPT) { if (err && err != LFS_ERR_CORRUPT) {
return err; return err;