From: Andreas Dilger Subject: Re: [PATCH V1 02/17] ext4: Add the basic function for inline data support. Date: Wed, 26 Oct 2011 02:36:56 -0600 Message-ID: References: <4EA7B788.3040503@tao.ma> <1319614468-11227-1-git-send-email-tm@tao.ma> <1319614468-11227-2-git-send-email-tm@tao.ma> Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: linux-ext4@vger.kernel.org, tytso@mit.edu, linux-kernel@vger.kernel.org To: Tao Ma Return-path: Received: from idcmail-mo2no.shaw.ca ([64.59.134.9]:57505 "EHLO idcmail-mo2no.shaw.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932494Ab1JZIhH (ORCPT ); Wed, 26 Oct 2011 04:37:07 -0400 In-Reply-To: <1319614468-11227-2-git-send-email-tm@tao.ma> Sender: linux-ext4-owner@vger.kernel.org List-ID: 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