Github/littlefs
1
Fork 0
Code Actions Activity

attrs: (Re)implemented file-attached custom attributes

Browse Source 
Unlike lfsr_setattr/getattr/etc, file-attached custom attributes are
RAM-backed snapshots attached to, well, files, that can be committed
atomically along with the file's contents. Great for power-loss
resilience, but boy does it make a mess of an API.

This API was really where custom attributes needed some TLC.

The biggest change is how file-attached custom attributes interact with
file sync broadcasting.

A common complaint from users is that setting custom attributes did not
update attributes in open file handles. This behavior is _very_
inconsistent with other filesystems and created a lot of confusion.
Since we're nailing down littlefs's snapshot/broadcasting model as a
part of larger changes, it makes sense to also nail down how custom
attributes interact.

In the new model:

- Custom attributes are still in-RAM snapshots. Updates do not
  immediately take effect, even across write calls.

- On lfsr_file_sync or lfsr_file_close, custom attributes are written
  atomically to disk and broadcasted to all open file handles.

- lfsr_setattr/removeattr also take part in attribute broadcasting. When
  called, lfsr_setattr/removeattr updates the attribute on disk and
  broadcasts the attribute changes to all open file handles.

- Desynced files do _not_ recieve any attribute broadcasts in the same
  way they do not recieve any data broadcasts.

This should hopefully make littlefs behave much more consistently with
other filesystems, while still maintaining a well-defined snapshot and
power-loss properties.

---

The lfs_attr struct also gained several new fields:

  // Custom attribute structure, used to describe custom attributes
  // committed atomically during file writes.
  struct lfs_attr {
      // Type of attribute
      //
      // Note some of this range is reserved:
      // 0x00-0x7f - Free for custom attributes
      // 0x80-0xff - May be assigned a standard attribute
      uint8_t type;

      // Flags that control how attr is read/written/removed
      uint8_t flags;

      // Pointer the buffer where the attr will be read/written
      void *buffer;

      // Size of the attr buffer in bytes, this can be set to
      // LFS_ERR_NOATTR to remove the attr
      lfs_ssize_t buffer_size;

      // Optional pointer to a mutable attr size, updated on read/write,
      // set to LFS_ERR_NOATTR if attr does not exist
      //
      // Defaults to buffer_size if NULL
      lfs_ssize_t *size;
  };

Which are useful for several new features:

- lfs_attr now supports LFS_A_RDONLY/WRONLY/RDWR modes.

  One of the blockers for attribute broadcasting was in-ROM attributes,
  where broadcast updates would hard-fault. But now if you mark in-ROM
  attributes as WRONLY, and in-RAM attributes as RDWR, this problem goes
  away.

- When opened, lfs_attr now optionally writes the attribute size to the
  indirect size field.

  No more hacky zero padding and not knowing an attribute's size.

  Note this follows the same rules as lfsr_getattr, so it does truncate
  if the buffer is too small.

  The size field can also be set to NULL, in which case lfs_attr
  defaults to the buffer_size. This can be quite useful for pure
  ROM-backed attributes.

- Missing attributes are now represented with size=LFS_ERR_NOATTR.

  No more zero-sized vs missing attribute ambiguity.

  This also makes it possible to remove attributes via lfs_attr, by
  setting the size to LFS_ERR_NOATTR manually.

  This does lead to a bit of a quirk where buffer_size can be
  LFS_ERR_NOATTR, which is a bit weird but at least consistent.

- Changes to lfs_attrs will now always trigger file syncs by default.

  Previously, if you changed an attribute, you had to also change the
  file's contents for it to get written to disk. As pointed out by users
  this is both surprising and difficult to work around.

  Solving this is quite tricky since there's no real signalling
  mechanism between attribute buffers and littlefs. The best I could
  come up with is to read attributes from disk during lfsr_file_sync to
  see if anything changed.

  At the very least, the new flag LFS_A_LAZY restores the old behavior
  in case the extra reads in lfsr_file_sync are problematic.

  Though I suspect _most_ calls to lfsr_file_sync immediately follow
  intentional changes to a file. It would be interesting to know of
  examples where this is not the case...

These new fields do increase the size of lfs_attr, which is a downside,
but thanks to flags fitting in type's padding, this is only an increase
from 3 words (12 bytes) -> 4 words (16 bytes).

---

Other implementation notes:

- I did try to implement LFS_A_CREAT/EXCL in lfs_attr but this proved
  to be too messy and inconsistent, so I dropped the idea for now.

  The idea was to error with NOATTR/EXIST if the lfs_attr flag in
  incompatible with what's on disk, but this led to a lot of complexity
  for what is a pretty niche use case.

  It's also inconsistent with rdonly attrs, which do _not_ error with
  NOATTR during lfsr_file_opencfg, because that would be kind of
  annoying.

- Having both `struct lfs_attr` and `lfsr_attr_t` to represent different
  things in the codebase is both fragile and confusing. One of these
  needs to change, probably `lfsr_attr_t`.

  If only I could think of a good name...

  One of the nice side-effects of the now-dropped uattr/sattr split was
  avoiding this conflict.

- We still need more tests related to how custom attributes interact
  with other filesystem operations, but I wanted to get what is
  currently working committed, see the TODOs in test_attrs.toml.

All of the new bells and whistles unfortunately do add up.
lfsr_file_sync is also the root of our current stack hot-path, so the
additional attr also adds a bit of stack:

           code          stack
  before: 37116           2608
  after:  38104 (+2.7%)   2624 (+0.6%)

Still, having a consistent and flexible API is well worth it.

Though I do think at some point we should add a compile-time option to
opt-out of custom attributes (LFS_NO_ATTR?).
This commit is contained in:
Christopher Haster
2024-08-22 16:41:06 -05:00
parent f539d3341c
commit f80db15c7e
3 changed files with 1360 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+300 -27
View File
@@ -1419,6 +1419,7 @@ enum lfsr_tag {
LFSR_TAG_SHRUBCOMMIT = 0x0801,
LFSR_TAG_SHRUBTRUNK = 0x0802,
LFSR_TAG_MOVE = 0x0803,
LFSR_TAG_UATTRS = 0x0804,
// some in-device only tag modifiers
LFSR_TAG_RM = 0x8000,
@@ -2394,6 +2395,11 @@ typedef struct lfsr_data_name {
#define LFSR_ATTR_ATTRS(_tag, _weight, _attrs, _attr_count) \
LFSR_ATTR_(_tag, _weight, (const lfsr_attr_t*){_attrs}, _attr_count)
// chain a list of user attributes, these require a bit of last-minute
// reencoding during commits
#define LFSR_ATTR_UATTRS(_tag, _weight, _attrs, _attr_count) \
LFSR_ATTR_(_tag, _weight, (const struct lfs_attr*){_attrs}, _attr_count)
// a move of all attrs from an mdir entry
#define LFSR_ATTR_MOVE(_tag, _weight, _mdir) \
LFSR_ATTR_(_tag, _weight, (const lfsr_mdir_t*){_mdir}, 0)
@@ -2419,6 +2425,41 @@ typedef struct lfsr_shrubcommit lfsr_shrubcommit_t;
LFSR_ATTR_(_tag, _weight, (const lfsr_shrub_t*){_shrub}, 0)
// operations on custom attribute lists
//
// a slightly different struct because it's user facing
static inline lfs_ssize_t lfsr_uattr_size(const struct lfs_attr *attr) {
// we default to the buffer_size if a mutable size is not provided
if (attr->size) {
return *attr->size;
} else {
return attr->buffer_size;
}
}
static inline bool lfsr_uattr_isnoattr(const struct lfs_attr *attr) {
return lfsr_uattr_size(attr) == LFS_ERR_NOATTR;
}
static lfs_scmp_t lfsr_uattr_cmp(lfs_t *lfs, const struct lfs_attr *attr,
const lfsr_data_t *data) {
// note data=NULL => NOATTR
if (!data) {
return (lfsr_uattr_isnoattr(attr)) ? LFS_CMP_EQ : LFS_CMP_GT;
} else {
if (lfsr_uattr_isnoattr(attr)) {
return LFS_CMP_LT;
} else {
return lfsr_data_cmp(lfs, *data,
attr->buffer,
lfsr_uattr_size(attr));
}
}
}
//struct lfsr_attr_from {
// const lfsr_rbyd_t *rbyd;
// const struct lfsr_attr *attrs;
@@ -7117,6 +7158,11 @@ static inline bool lfsr_o_iszombie(uint32_t flags) {
return flags & LFS_O_ZOMBIE;
}
// custom attr flags
static inline bool lfsr_a_islazy(uint32_t flags) {
return flags & LFS_A_LAZY;
}
// traversal flags
static inline bool lfsr_t_ismtreeonly(uint32_t flags) {
return flags & LFS_T_MTREEONLY;
@@ -8135,6 +8181,57 @@ static int lfsr_mdir_commit__(lfs_t *lfs, lfsr_mdir_t *mdir,
}
}
// custom attributes need to be reencoded into our tag format
} else if (lfsr_tag_key(attrs[i].tag) == LFSR_TAG_UATTRS) {
const struct lfs_attr *attrs_ = attrs[i].cat;
lfs_size_t attr_count_ = attrs[i].count;
for (lfs_size_t j = 0; j < attr_count_; j++) {
// skip readonly attrs and lazy attrs
if (lfsr_o_isrdonly(attrs_[j].flags)) {
continue;
}
// first lets check if the attr changed, we don't want
// to append attrs unless we have to
lfsr_data_t data;
int err = lfsr_mdir_lookup(lfs, mdir,
LFSR_TAG_ATTR(attrs_[j].type),
&data);
if (err && err != LFS_ERR_NOENT) {
return err;
}
// does disk match our attr?
lfs_scmp_t cmp = lfsr_uattr_cmp(lfs, &attrs_[j],
(err != LFS_ERR_NOENT) ? &data : NULL);
if (cmp < 0) {
return cmp;
}
if (cmp == LFS_CMP_EQ) {
continue;
}
// append the custom attr
err = lfsr_rbyd_appendattr(lfs, &mdir->rbyd,
rid - lfs_smax(start_rid, 0),
// removing or updating?
(lfsr_uattr_isnoattr(&attrs_[j]))
? LFSR_ATTR(
LFSR_TAG_RM
| LFSR_TAG_ATTR(attrs_[j].type), 0,
LFSR_DATA_NULL())
: LFSR_ATTR(
LFSR_TAG_ATTR(attrs_[j].type), 0,
LFSR_DATA_BUF(
attrs_[j].buffer,
lfsr_uattr_size(&attrs_[j]))));
if (err) {
return err;
}
}
// write out normal tags normally
} else {
LFS_ASSERT(!lfsr_tag_isinternal(attrs[i].tag));
@@ -11149,10 +11246,39 @@ int lfsr_setattr(lfs_t *lfs, const char *path, uint8_t type,
// commit our attr
lfs_alloc_ckpoint(lfs);
return lfsr_mdir_commit(lfs, &mdir, LFSR_ATTRS(
err = lfsr_mdir_commit(lfs, &mdir, LFSR_ATTRS(
LFSR_ATTR(
LFSR_TAG_ATTR(type), 0,
LFSR_DATA_BUF(buffer, size))));
if (err) {
return err;
}
// update any opened files tracking custom attrs
for (lfsr_omdir_t *o = lfs->omdirs; o; o = o->next) {
if (!(lfsr_o_type(o->flags) == LFS_TYPE_REG
&& o->mdir.mid == mdir.mid
&& !lfsr_o_isdesync(o->flags))) {
continue;
}
lfsr_file_t *file = (lfsr_file_t*)o;
for (lfs_size_t i = 0; i < file->cfg->attr_count; i++) {
if (!(file->cfg->attrs[i].type == type
&& !lfsr_o_iswronly(file->cfg->attrs[i].flags))) {
continue;
}
lfs_size_t d = lfs_min(size, file->cfg->attrs[i].buffer_size);
memcpy(file->cfg->attrs[i].buffer, buffer, d);
if (file->cfg->attrs[i].size) {
*file->cfg->attrs[i].size = d;
}
}
}
return 0;
}
int lfsr_removeattr(lfs_t *lfs, const char *path, uint8_t type) {
@@ -11172,10 +11298,36 @@ int lfsr_removeattr(lfs_t *lfs, const char *path, uint8_t type) {
// commit our removal
lfs_alloc_ckpoint(lfs);
return lfsr_mdir_commit(lfs, &mdir, LFSR_ATTRS(
err = lfsr_mdir_commit(lfs, &mdir, LFSR_ATTRS(
LFSR_ATTR(
LFSR_TAG_RM | LFSR_TAG_ATTR(type), 0,
LFSR_DATA_NULL())));
if (err) {
return err;
}
// update any opened files tracking custom attrs
for (lfsr_omdir_t *o = lfs->omdirs; o; o = o->next) {
if (!(lfsr_o_type(o->flags) == LFS_TYPE_REG
&& o->mdir.mid == mdir.mid
&& !lfsr_o_isdesync(o->flags))) {
continue;
}
lfsr_file_t *file = (lfsr_file_t*)o;
for (lfs_size_t i = 0; i < file->cfg->attr_count; i++) {
if (!(file->cfg->attrs[i].type == type
&& !lfsr_o_iswronly(file->cfg->attrs[i].flags))) {
continue;
}
if (file->cfg->attrs[i].size) {
*file->cfg->attrs[i].size = LFS_ERR_NOATTR;
}
}
}
return 0;
}
@@ -11292,6 +11444,43 @@ static int lfsr_file_fetch(lfs_t *lfs, lfsr_file_t *file) {
file->buffer.size = size;
}
// try to fetch any custom attributes
for (lfs_size_t i = 0; i < file->cfg->attr_count; i++) {
// skip writeonly attrs
if (lfsr_o_iswronly(file->cfg->attrs[i].flags)) {
continue;
}
// lookup the attr
lfsr_data_t data;
int err = lfsr_mdir_lookup(lfs, &file->o.o.mdir,
LFSR_TAG_ATTR(file->cfg->attrs[i].type),
&data);
if (err && err != LFS_ERR_NOENT) {
return err;
}
// read the attr, if it exists
if (err == LFS_ERR_NOENT
// awkward case here if buffer_size is LFS_ERR_NOATTR
|| file->cfg->attrs[i].buffer_size == LFS_ERR_NOATTR) {
if (file->cfg->attrs[i].size) {
*file->cfg->attrs[i].size = LFS_ERR_NOATTR;
}
} else {
lfs_ssize_t d = lfsr_data_read(lfs, &data,
file->cfg->attrs[i].buffer,
file->cfg->attrs[i].buffer_size);
if (d < 0) {
return d;
}
if (file->cfg->attrs[i].size) {
*file->cfg->attrs[i].size = d;
}
}
}
return 0;
}
@@ -11324,6 +11513,13 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file,
LFS_ASSERT(!lfsr_o_isrdonly(flags) || !lfsr_o_iscreat(flags));
LFS_ASSERT(!lfsr_o_isrdonly(flags) || !lfsr_o_isexcl(flags));
LFS_ASSERT(!lfsr_o_isrdonly(flags) || !lfsr_o_istrunc(flags));
for (lfs_size_t i = 0; i < cfg->attr_count; i++) {
// these flags require a writable attr
LFS_ASSERT(!lfsr_o_isrdonly(cfg->attrs[i].flags)
|| !lfsr_o_iscreat(cfg->attrs[i].flags));
LFS_ASSERT(!lfsr_o_isrdonly(cfg->attrs[i].flags)
|| !lfsr_o_isexcl(cfg->attrs[i].flags));
}
if (!lfsr_o_isrdonly(flags)) {
// prepare our filesystem for writing
@@ -12810,30 +13006,34 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
LFS_ASSERT(!lfsr_o_isorphan(file->o.o.flags)
|| lfsr_o_isunsync(file->o.o.flags));
// don't write to disk if already in-sync
if (lfsr_o_isunsync(file->o.o.flags)) {
// commit any changes to our file's metadata
lfsr_attr_t attrs[2];
lfs_size_t attr_count = 0;
lfsr_data_t name_data;
uint8_t buf[LFSR_BTREE_DSIZE];
// build a commit of any pending file metadata
lfsr_attr_t attrs[3];
lfs_size_t attr_count = 0;
lfsr_data_t name_data;
uint8_t buf[LFSR_BTREE_DSIZE];
// not created yet? need to convert orphan to normal file
if (lfsr_o_isorphan(file->o.o.flags)) {
err = lfsr_mdir_lookup(lfs, &file->o.o.mdir, LFSR_TAG_ORPHAN,
&name_data);
if (err) {
// we must have an orphan at this point
LFS_ASSERT(err != LFS_ERR_NOENT);
goto failed;
}
attrs[attr_count++] = LFSR_ATTR_CAT_(
LFSR_TAG_SUB | LFSR_TAG_REG, 0,
&name_data, 1);
// not created yet? need to convert orphan to normal file
if (lfsr_o_isorphan(file->o.o.flags)) {
err = lfsr_mdir_lookup(lfs, &file->o.o.mdir, LFSR_TAG_ORPHAN,
&name_data);
if (err) {
// orphan flag but no orphan tag?
LFS_ASSERT(err != LFS_ERR_NOENT);
goto failed;
}
// commit the file state
attrs[attr_count++] = LFSR_ATTR_CAT_(
LFSR_TAG_SUB | LFSR_TAG_REG, 0,
&name_data, 1);
}
// pending file changes?
if (lfsr_o_isunsync(file->o.o.flags)) {
// make sure data is on-disk before committing metadata
err = lfsr_bd_sync(lfs);
if (err) {
goto failed;
}
// null? no attr?
if (lfsr_o_isunflush(file->o.o.flags) && file->buffer.size == 0) {
@@ -12858,13 +13058,54 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
} else {
LFS_UNREACHABLE();
}
}
// make sure data is on-disk before committing metadata
err = lfsr_bd_sync(lfs);
if (err) {
goto failed;
// pending custom attributes?
//
// this gets real messy, since users can change custom attributes
// whenever they want without informing littlefs, the best we can do
// is read from disk to manually check if any attributes changed
bool uattrs = lfsr_o_isunsync(file->o.o.flags);
if (!uattrs) {
for (lfs_size_t i = 0; i < file->cfg->attr_count; i++) {
// skip readonly attrs and lazy attrs
if (lfsr_o_isrdonly(file->cfg->attrs[i].flags)
|| lfsr_a_islazy(file->cfg->attrs[i].flags)) {
continue;
}
// lookup the attr
lfsr_data_t data;
err = lfsr_mdir_lookup(lfs, &file->o.o.mdir,
LFSR_TAG_ATTR(file->cfg->attrs[i].type),
&data);
if (err && err != LFS_ERR_NOENT) {
goto failed;
}
// does disk match our attr?
lfs_scmp_t cmp = lfsr_uattr_cmp(lfs, &file->cfg->attrs[i],
(err != LFS_ERR_NOENT) ? &data : NULL);
if (cmp < 0) {
err = cmp;
goto failed;
}
if (cmp != LFS_CMP_EQ) {
uattrs = true;
break;
}
}
}
if (uattrs) {
// need to append custom attributes
attrs[attr_count++] = LFSR_ATTR_UATTRS(
LFSR_TAG_UATTRS, 0,
file->cfg->attrs, file->cfg->attr_count);
}
// pending metadata? looks like we need to write to disk
if (attr_count > 0) {
// checkpoint the allocator
lfs_alloc_ckpoint(lfs);
@@ -12907,6 +13148,38 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
file->buffer.buffer,
file->buffer.size);
file_->buffer.size = file->buffer.size;
// update any custom attrs
for (lfs_size_t i = 0; i < file->cfg->attr_count; i++) {
if (lfsr_o_isrdonly(file->cfg->attrs[i].flags)) {
continue;
}
for (lfs_size_t j = 0; j < file_->cfg->attr_count; j++) {
if (!(file_->cfg->attrs[j].type
== file->cfg->attrs[i].type
&& !lfsr_o_iswronly(
file_->cfg->attrs[j].flags))) {
continue;
}
if (lfsr_uattr_isnoattr(&file->cfg->attrs[i])) {
if (file_->cfg->attrs[j].size) {
*file_->cfg->attrs[j].size = LFS_ERR_NOATTR;
}
} else {
lfs_size_t d = lfs_min(
lfsr_uattr_size(&file->cfg->attrs[i]),
file_->cfg->attrs[j].buffer_size);
memcpy(file_->cfg->attrs[j].buffer,
file->cfg->attrs[i].buffer,
d);
if (file_->cfg->attrs[j].size) {
*file_->cfg->attrs[j].size = d;
}
}
}
}
}
// clobber entangled traversals
lfs.h
+40
View File
@@ -152,8 +152,12 @@ enum lfs_type {
#define LFS_SEEK_END 2 // Seek relative to the end of the file
// Custom attribute flags
#define LFS_A_RDONLY 0 // Open an attr as read only
#define LFS_A_WRONLY 1 // Open an attr as write only
#define LFS_A_RDWR 2 // Open an attr as read and write
#define LFS_A_CREAT 0x04 // Create an attr if it does not exist
#define LFS_A_EXCL 0x08 // Fail if an attr already exists
#define LFS_A_LAZY 0x10 // Only write attr if file changed
// Filesystem format flags
#define LFS_F_RDWR 0 // Format the filesystem as read and write
@@ -481,6 +485,33 @@ struct lfs_tinfo {
// lfs_size_t size;
//};
// Custom attribute structure, used to describe custom attributes
// committed atomically during file writes.
struct lfs_attr {
// Type of attribute
//
// Note some of this range is reserved:
// 0x00-0x7f - Free for custom attributes
// 0x80-0xff - May be assigned a standard attribute
uint8_t type;
// Flags that control how attr is read/written/removed
uint8_t flags;
// Pointer the buffer where the attr will be read/written
void *buffer;
// Size of the attr buffer in bytes, this can be set to
// LFS_ERR_NOATTR to remove the attr
lfs_ssize_t buffer_size;
// Optional pointer to a mutable attr size, updated on read/write,
// set to LFS_ERR_NOATTR if attr does not exist
//
// Defaults to buffer_size if NULL
lfs_ssize_t *size;
};
// Optional configuration provided during lfs_file_opencfg
struct lfs_file_config {
// Optional statically allocated file buffer. Must be buffer_size.
@@ -507,6 +538,15 @@ struct lfs_file_config {
//
// // Number of custom attributes in the list
// lfs_size_t attr_count;
// Optional list of custom attributes attached to the file. If readable,
// these attributes will be kept up to date with the attributes on-disk.
// If writeable, these attributes will be written to disk atomically on
// every file sync or close.
struct lfs_attr *attrs;
// Number of custom attributes in the list
lfs_size_t attr_count;
};
tests/test_attrs.toml
+1020 -24
View File
File diff suppressed because it is too large Load Diff