On Wed, Jun 14, 2017 at 10:23:26AM -0700, Tahsin Erdogan wrote: > New ea_inode feature allows putting large xattr values into external > inodes. struct ext4_xattr_entry and the attribute name however have to > remain in the inode extra space or external attribute block. Once that > space is exhausted, no further entries can be added. Some of that space > could also be used by values that fit in there at the time of addition. > > So, a single xattr entry whose value barely fits in the external block > could prevent further entries being added. > > To mitigate the problem, this patch introduces a notion of reserve in > the > external attribute block that cannot be used by value data. This reserve > is enforced when ea_inode feature is enabled. The amount of reserve is > arbitrarily chosen to be min(block_size/8, 1024). The table below shows > how much space is reserved for each block size and the guaranteed > mininum > number of entries that can be placed in the external attribute block. > > block size reserved bytes entries (name length = 16) > 1k 128 3 > 2k 256 7 > 4k 512 15
Why not just spill the values into their own ea_inodes if we need the space? I guess that has the disadvantage that now we need to reserve quite a few more journal credits ((1 inode block, 1 bbitmap block, 1 ibitmap block, 1 data block) * nr_inline_values) just in case we end up spilling all the values. --D > 8k 1024 31 > 16k 1024 31 > 32k 1024 31 > 64k 1024 31 > > Signed-off-by: Tahsin Erdogan <tah...@google.com> > --- > fs/ext4/xattr.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c > index 3ad1fc62cbf0..c9579d220a0c 100644 > --- a/fs/ext4/xattr.c > +++ b/fs/ext4/xattr.c > @@ -1428,6 +1428,12 @@ static int ext4_xattr_inode_lookup_create(handle_t > *handle, struct inode *inode, > return 0; > } > > +/* > + * Reserve min(block_size/8, 1024) bytes for xattr entries/names if ea_inode > + * feature is enabled. > + */ > +#define EXT4_XATTR_BLOCK_RESERVE(inode) min(i_blocksize(inode)/8, 1024U) > + > static int ext4_xattr_set_entry(struct ext4_xattr_info *i, > struct ext4_xattr_search *s, > handle_t *handle, struct inode *inode) > @@ -1487,6 +1493,20 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info > *i, > ret = -ENOSPC; > goto out; > } > + > + /* > + * If storing the value in an external inode is an option, > + * reserve space for xattr entries/names in the external > + * attribute block so that a long value does not occupy the > + * whole space and prevent futher entries being added. > + */ > + if (ext4_has_feature_ea_inode(inode->i_sb) && new_size && > + (s->end - s->base) == i_blocksize(inode) && > + (min_offs + old_size - new_size) < > + EXT4_XATTR_BLOCK_RESERVE(inode)) { > + ret = -ENOSPC; > + goto out; > + } > } > > /* > -- > 2.13.1.508.gb3defc5cc-goog > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Jfs-discussion mailing list Jfs-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jfs-discussion