From 6c754c802352d7c46b8b10ba4dd8088b2cea2d86 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 5 Apr 2018 23:23:14 -0500 Subject: [PATCH] Added support for atomically committing custom attributes Although it's simple and probably what most users expect, the previous custom attributes API suffered from one problem: the inability to update attributes atomically. If we consider our timestamp use case, updating a file would require: 1. Update the file 2. Update the timestamp If a power loss occurs during this sequence of updates, we could end up with a file with an incorrect timestamp. Is this a big deal? Probably not, but it could be a surprise only found after a power-loss. And littlefs was developed with the _specifically_ to avoid suprises during power-loss. The littlefs is perfectly capable of bundling multiple attribute updates in a single directory commit. That's kind of what it was designed to do. So all we need is a new committer opcode for list of attributes, and then poking that list of attributes through the API. We could provide the single-attribute functions, but don't, because the fewer functions makes for a smaller codebase, and these are already the more advanced functions so we can expect more from users. This also changes semantics about what happens when we don't find an attribute, since erroring would throw away all of the other attributes we're processing. To atomically commit both custom attributes and file updates, we need a new API, lfs_file_setattr. Unfortunately the semantics are a bit more confusing than lfs_setattr, since the attributes aren't written out immediately. --- lfs.c | 254 ++++++++++++++++++++++++++++++++++------------------------ lfs.h | 94 +++++++++++++++++----- 2 files changed, 224 insertions(+), 124 deletions(-) diff --git a/lfs.c b/lfs.c index fa63e8e..b079ef4 100644 --- a/lfs.c +++ b/lfs.c @@ -515,6 +515,7 @@ struct lfs_region { LFS_FROM_DROP, LFS_FROM_MEM, LFS_FROM_REGION, + LFS_FROM_ATTRS, } type; lfs_off_t off; @@ -522,6 +523,12 @@ struct lfs_region { lfs_ssize_t size; }; +struct lfs_attrs_region { + const struct lfs_attr *attrs; + int count; + lfs_size_t len; +}; + struct lfs_region_region { lfs_block_t block; lfs_off_t off; @@ -568,6 +575,73 @@ static int lfs_commit_region(lfs_t *lfs, newoff += regions[i].size; break; } + case LFS_FROM_ATTRS: { + const struct lfs_attrs_region *attrs = regions[i].buffer; + + // order doesn't matter, so we write new attrs first. this + // is still O(n^2) but only O(n) disk access + for (int j = 0; j < attrs->count; j++) { + if (attrs->attrs[j].size == 0) { + continue; + } + + lfs_entry_attr_t attr = { + .d.type = attrs->attrs[j].type, + .d.len = attrs->attrs[j].size, + }; + + lfs_crc(crc, &attr.d, sizeof(attr.d)); + int err = lfs_bd_prog(lfs, newblock, newoff, + &attr.d, sizeof(attr.d)); + if (err) { + return err; + } + + lfs_crc(crc, + attrs->attrs[j].buffer, attrs->attrs[j].size); + err = lfs_bd_prog(lfs, newblock, newoff+sizeof(attr.d), + attrs->attrs[j].buffer, attrs->attrs[j].size); + if (err) { + return err; + } + + newoff += sizeof(attr.d) + attrs->attrs[j].size; + } + + // copy over attributes without updates + lfs_entry_attr_t attr; + for (lfs_off_t k = 0; k < attrs->len; k += 2+attr.d.len) { + int err = lfs_bd_read(lfs, oldblock, oldoff, + &attr.d, sizeof(attr.d)); + if (err) { + return err; + } + + bool updating = false; + for (int j = 0; j < attrs->count; j++) { + if (attr.d.type == attrs->attrs[j].type) { + updating = true; + } + } + + if (!updating) { + err = lfs_commit_region(lfs, + oldblock, oldoff, + newblock, newoff, + 0, NULL, 0, + attr.d.len, crc); + if (err) { + return err; + } + + newoff += 2+attr.d.len; + } + + oldoff += 2+attr.d.len; + } + + break; + } } i += 1; @@ -590,6 +664,8 @@ static int lfs_commit_region(lfs_t *lfs, } } + // sanity check our commit math + LFS_ASSERT(newoff == end); return 0; } @@ -1044,116 +1120,100 @@ static int lfs_dir_getinfo(lfs_t *lfs, return 0; } -static int lfs_dir_getattr(lfs_t *lfs, +static int lfs_dir_getattrs(lfs_t *lfs, lfs_dir_t *dir, const lfs_entry_t *entry, - uint8_t type, void *buffer, lfs_size_t size) { + const struct lfs_attr *attrs, int count) { + // set to zero in case we can't find the attributes or size mismatch + for (int j = 0; j < count; j++) { + memset(attrs[j].buffer, 0, attrs[j].size); + } + // search for attribute in attribute region - lfs_off_t off = sizeof(dir->d) + lfs_entry_elen(entry); - lfs_off_t i = 0; - while (i < lfs_entry_alen(entry)) { - lfs_attr_t attr; + lfs_off_t off = 4+lfs_entry_elen(entry); + lfs_entry_attr_t attr; + for (lfs_off_t i = 0; i < lfs_entry_alen(entry); i += 2+attr.d.len) { int err = lfs_dir_get(lfs, dir, entry->off+off+i, &attr.d, sizeof(attr.d)); if (err) { return err; } - if (attr.d.type != type) { - i += attr.d.len; - continue; + for (int j = 0; j < count; j++) { + if (attr.d.type == attrs[j].type) { + if (attr.d.len > attrs[j].size) { + return LFS_ERR_RANGE; + } + + err = lfs_dir_get(lfs, dir, + entry->off+off+i+sizeof(attr.d), + attrs[j].buffer, attr.d.len); + if (err) { + return err; + } + } } - - if (attr.d.len > size) { - return LFS_ERR_RANGE; - } - - err = lfs_dir_get(lfs, dir, - entry->off+off+i+sizeof(attr.d), buffer, attr.d.len); - if (err) { - return err; - } - - return attr.d.len; - } - - return LFS_ERR_NODATA; -} - -static int lfs_dir_setattr(lfs_t *lfs, - lfs_dir_t *dir, lfs_entry_t *entry, - uint8_t type, const void *buffer, lfs_size_t size) { - // search for attribute in attribute region - lfs_off_t off = sizeof(dir->d) + lfs_entry_elen(entry); - lfs_off_t i = 0; - lfs_size_t oldlen = 0; - while (i < lfs_entry_alen(entry)) { - lfs_attr_t attr; - int err = lfs_dir_get(lfs, dir, - entry->off+off+i, &attr.d, sizeof(attr.d)); - if (err) { - return err; - } - - if (attr.d.type != type) { - i += attr.d.len; - continue; - } - - oldlen = attr.d.len; - break; - } - - // make sure the attribute fits - if (lfs_entry_elen(entry) - oldlen + size > lfs->attrs_size || - (0x7fffffff & dir->d.size) - oldlen + size > lfs->cfg->block_size) { - return LFS_ERR_NOSPC; - } - - lfs_attr_t attr; - attr.d.type = type; - attr.d.len = size; - int err = lfs_dir_set(lfs, dir, entry, (struct lfs_region[]){ - {LFS_FROM_MEM, off+i, &attr.d, sizeof(attr.d)}, - {LFS_FROM_MEM, off+i, buffer, size}, - {LFS_FROM_DROP, off+i, NULL, -oldlen}}, 3); - if (err) { - return err; } return 0; } -static int lfs_dir_removeattr(lfs_t *lfs, - lfs_dir_t *dir, lfs_entry_t *entry, uint8_t type) { - // search for attribute in attribute region - lfs_off_t off = sizeof(dir->d) + lfs_entry_elen(entry); - lfs_off_t i = 0; - while (i < lfs_entry_alen(entry)) { - lfs_attr_t attr; +static lfs_ssize_t lfs_dir_checkattrs(lfs_t *lfs, + lfs_dir_t *dir, lfs_entry_t *entry, + const struct lfs_attr *attrs, int count) { + // check that attributes fit + lfs_size_t nsize = 0; + for (int j = 0; j < count; j++) { + nsize += 2+attrs[j].size; + } + + lfs_off_t off = 4+lfs_entry_elen(entry); + lfs_entry_attr_t attr; + for (lfs_off_t i = 0; i < lfs_entry_alen(entry); i += 2+attr.d.len) { int err = lfs_dir_get(lfs, dir, entry->off+off+i, &attr.d, sizeof(attr.d)); if (err) { return err; } - if (attr.d.type != type) { - i += attr.d.len; - continue; + bool updated = false; + for (int j = 0; j < count; j++) { + if (attr.d.type == attrs[j].type) { + updated = true; + } } - err = lfs_dir_set(lfs, dir, entry, (struct lfs_region[]){ - {LFS_FROM_DROP, off+i, - NULL, -(sizeof(attr.d)+attr.d.len)}}, 1); - if (err) { - return err; + if (!updated) { + nsize += 2+attr.d.len; } - - return 0; } - return LFS_ERR_NODATA; + if (nsize > lfs->attrs_size || ( + (0x7fffffff & dir->d.size) + lfs_entry_size(entry) - + lfs_entry_alen(entry) + nsize > lfs->cfg->block_size)) { + return LFS_ERR_NOSPC; + } + + return nsize; } +static int lfs_dir_setattrs(lfs_t *lfs, + lfs_dir_t *dir, lfs_entry_t *entry, + const struct lfs_attr *attrs, int count) { + // make sure attributes fit + lfs_ssize_t nsize = lfs_dir_checkattrs(lfs, dir, entry, attrs, count); + if (nsize < 0) { + return nsize; + } + + // commit to entry, majority of work is in LFS_FROM_ATTRS + lfs_size_t oldlen = lfs_entry_alen(entry); + entry->d.alen = (0xc0 & entry->d.alen) | nsize; + return lfs_dir_set(lfs, dir, entry, (struct lfs_region[]){ + {LFS_FROM_MEM, 0, &entry->d, 4}, + {LFS_FROM_DROP, 0, NULL, -4}, + {LFS_FROM_ATTRS, 4+lfs_entry_elen(entry), + &(struct lfs_attrs_region){attrs, count, oldlen}, nsize}}, 3); +} /// Top level directory operations /// @@ -2372,8 +2432,8 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { /// Attribute operations /// -int lfs_getattr(lfs_t *lfs, const char *path, - uint8_t type, void *buffer, lfs_size_t size) { +int lfs_getattrs(lfs_t *lfs, const char *path, + const struct lfs_attr *attrs, int count) { lfs_dir_t cwd; int err = lfs_dir_fetch(lfs, &cwd, lfs->root); if (err) { @@ -2386,11 +2446,11 @@ int lfs_getattr(lfs_t *lfs, const char *path, return err; } - return lfs_dir_getattr(lfs, &cwd, &entry, type, buffer, size); + return lfs_dir_getattrs(lfs, &cwd, &entry, attrs, count); } -int lfs_setattr(lfs_t *lfs, const char *path, - uint8_t type, const void *buffer, lfs_size_t size) { +int lfs_setattrs(lfs_t *lfs, const char *path, + const struct lfs_attr *attrs, int count) { lfs_dir_t cwd; int err = lfs_dir_fetch(lfs, &cwd, lfs->root); if (err) { @@ -2403,23 +2463,7 @@ int lfs_setattr(lfs_t *lfs, const char *path, return err; } - return lfs_dir_setattr(lfs, &cwd, &entry, type, buffer, size); -} - -int lfs_removeattr(lfs_t *lfs, const char *path, uint8_t type) { - lfs_dir_t cwd; - int err = lfs_dir_fetch(lfs, &cwd, lfs->root); - if (err) { - return err; - } - - lfs_entry_t entry; - err = lfs_dir_find(lfs, &cwd, &entry, &path); - if (err) { - return err; - } - - return lfs_dir_removeattr(lfs, &cwd, &entry, type); + return lfs_dir_setattrs(lfs, &cwd, &entry, attrs, count); } diff --git a/lfs.h b/lfs.h index adc1c02..9de7a3a 100644 --- a/lfs.h +++ b/lfs.h @@ -234,6 +234,18 @@ struct lfs_info { char name[LFS_NAME_MAX+1]; }; +// Custom attribute structure +struct lfs_attr { + // Type of attribute, provided by user and used to identify the attribute + uint8_t type; + + // Pointer to buffer containing the attribute + void *buffer; + + // Size of attribute in bytes, limited to LFS_ATTRS_MAX + lfs_size_t size; +}; + /// littlefs data structures /// typedef struct lfs_entry { @@ -255,12 +267,12 @@ typedef struct lfs_entry { } d; } lfs_entry_t; -typedef struct lfs_attr { - struct lfs_disk_attr { +typedef struct lfs_entry_attr { + struct lfs_disk_entry_attr { uint8_t type; uint8_t len; } d; -} lfs_attr_t; +} lfs_entry_attr_t; typedef struct lfs_cache { lfs_block_t block; @@ -388,28 +400,25 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath); // Returns a negative error code on failure. int lfs_stat(lfs_t *lfs, const char *path, struct lfs_info *info); -// Get a custom attribute +// Get custom attributes // -// Attributes are identified by an 8-bit type and are limited to at -// most LFS_ATTRS_SIZE bytes. -// Returns the size of the attribute, or a negative error code on failure. -int lfs_getattr(lfs_t *lfs, const char *path, - uint8_t type, void *buffer, lfs_size_t size); - -// Set a custom attribute +// Attributes are looked up based on the type id. If the stored attribute is +// smaller than the buffer, it is padded with zeros. It the stored attribute +// is larger than the buffer, LFS_ERR_RANGE is returned. // -// Attributes are identified by an 8-bit type and are limited to at -// most LFS_ATTRS_SIZE bytes. // Returns a negative error code on failure. -int lfs_setattr(lfs_t *lfs, const char *path, - uint8_t type, const void *buffer, lfs_size_t size); +int lfs_getattrs(lfs_t *lfs, const char *path, + const struct lfs_attr *attrs, int count); -// Remove a custom attribute +// Set custom attributes +// +// The array of attributes will be used to update the attributes stored on +// disk based on their type id. Unspecified attributes are left unmodified. +// Specifying an attribute with zero size deletes the attribute. // -// Attributes are identified by an 8-bit type and are limited to at -// most LFS_ATTRS_SIZE bytes. // Returns a negative error code on failure. -int lfs_removeattr(lfs_t *lfs, const char *path, uint8_t type); +int lfs_setattrs(lfs_t *lfs, const char *path, + const struct lfs_attr *attrs, int count); /// File operations /// @@ -484,6 +493,30 @@ int lfs_file_rewind(lfs_t *lfs, lfs_file_t *file); // Returns the size of the file, or a negative error code on failure. lfs_soff_t lfs_file_size(lfs_t *lfs, lfs_file_t *file); +// Get custom attributes attached to a file +// +// Attributes are looked up based on the type id. If the stored attribute is +// smaller than the buffer, it is padded with zeros. It the stored attribute +// is larger than the buffer, LFS_ERR_RANGE is returned. +// +// Returns a negative error code on failure. +int lfs_file_getattrs(lfs_t *lfs, lfs_file_t *file, + const struct lfs_attr *attrs, int count); + +// Set custom attributes on a file +// +// The array of attributes will be used to update the attributes stored on +// disk based on their type id. Unspecified attributes are left unmodified. +// Specifying an attribute with zero size deletes the attribute. +// +// Note: Attributes are not written out until a call to lfs_file_sync +// or lfs_file_close and must be allocated until the file is closed or +// lfs_file_setattrs is called with a count of zero. +// +// Returns a negative error code on failure. +int lfs_file_setattrs(lfs_t *lfs, lfs_file_t *file, + const struct lfs_attr *attrs, int count); + /// Directory operations /// @@ -532,6 +565,29 @@ lfs_soff_t lfs_dir_tell(lfs_t *lfs, lfs_dir_t *dir); int lfs_dir_rewind(lfs_t *lfs, lfs_dir_t *dir); +/// Filesystem filesystem operations /// + +// Get custom attributes on the filesystem +// +// Attributes are looked up based on the type id. If the stored attribute is +// smaller than the buffer, it is padded with zeros. It the stored attribute +// is larger than the buffer, LFS_ERR_RANGE is returned. +// +// Returns a negative error code on failure. +int lfs_fs_getattrs(lfs_t *lfs, const struct lfs_attr *attrs, int count); + +// Set custom attributes on the filesystem +// +// The array of attributes will be used to update the attributes stored on +// disk based on their type id. Unspecified attributes are left unmodified. +// Specifying an attribute with zero size deletes the attribute. +// +// Note: Filesystem level attributes are not available for wear-leveling +// +// Returns a negative error code on failure. +int lfs_fs_setattrs(lfs_t *lfs, const struct lfs_attr *attrs, int count); + + /// Miscellaneous littlefs specific operations /// // Traverse through all blocks in use by the filesystem