Re: [PATCH V1 02/17] ext4: Add the basic function for inline data support.

[Andreas Dilger]

On 2011-10-26, at 1:34 AM, Tao Ma wrote:
> Implement inline data with xattr. This idea is inspired by Andreas.
> So now we use "system.data" to store xattr.  For inode_size = 256,
> currently we uses all the space between i_extra_isize and inode_size.
> For inode_size > 256, we use half of that space.
> 
> +#define EXT4_XATTR_SYSTEM_DATA_NAME		"data"

Did you check whether the "data" string takes 4 bytes or 8 in the
xattr header?  I believe it is storing the NUL termination (and
the code is using "sizeof(EXT4_XATTR_SYSTEM_DATA_NAME)" = 5), so
I believe it will round up to the next 4-byte boundary.  Using a
3-byte name will save a bit of space, like "system.dat".

> +void ext4_write_inline_data(struct inode *inode, struct ext4_iloc *iloc,
> +			    void *buffer, loff_t pos, unsigned len)
> +{
> +	header = IHDR(inode, ext4_raw_inode(iloc));
> +	entry = (struct ext4_xattr_entry *)((void *)ext4_raw_inode(iloc) +
> +					    EXT4_I(inode)->i_inline_off);
> +	memcpy((void *)IFIRST(header) + le16_to_cpu(entry->e_value_offs) + pos,
> +	       buffer + pos, len);
> +}
> +
> +int ext4_init_inline_data(handle_t *handle, struct inode *inode,
> +			  struct ext4_iloc *iloc)
> +{
> +	size = ext4_get_max_inline_size(inode);
> +	value = kzalloc(size, GFP_NOFS);
> +	if (!value)
> +		return -ENOMEM;
> +
> +	error = ext4_xattr_ibody_set(handle, inode, &i, &is);
> +}

Since file data is changed very rarely, instead of consuming the full
xattr space that may not be needed, wouldn't it be better to change
ext4_write_inline_data() to just to the ext4_xattr_ibody_set() to save
the exact-sized buffer into the xattr?  That will allow other xattrs
to be stored in this space as well as the inline data.


Cheers, Andreas





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/