在 2008-08-25一的 16:47 -0600，Andreas Dilger写道：
> On Aug 25, 2008 21:30 +0530, Kalpak Shah wrote:
> > This is the implementation for large EA support in ext4. Note that this
> > also helps to have a larger number of EAs since large EAs get written
> > out to a new inode instead of the EA block.
> >
> > If value of an attribute is greater than 2048 bytes the value is not
> > saved in the external EA block, instead it is saved in an inode.
>
> I just realized that this needs to be (blocksize / 2) instead of 2048,
> or we will never get the EA inodes in case of 1kB/2kB block filesystem
> where we need it the most.
>
> > +struct inode *ext4_xattr_inode_iget(struct inode *parent, int ea_ino, int *err)
> ^^ extra space here
>
> > + if (ea_inode->i_mtime.tv_sec != parent->i_ino ||
>
> Do you think it makes sense to "#define i_xattr_inode_parent i_mtime.tv_sec"
> in case there is a decision to change which field is used? Or do people
> think that is more confusing than helpful?
>
> Note to readers that this is a new patch, and Lustre doesn't use it yet,
> but we'd like to in the relatively near future so feedback that affects
> the on disk format is preferred sooner than later.
>
> > +ext4_xattr_inode_set(handle_t *handle, struct inode *inode, int *ea_ino,
> > + const void *value, size_t value_len)
> > +{
> > + /*
> > + * Make sure that enough buffer credits are available else extend the
> > + * transaction.
> > + */
> > + req_buffer_credits = (value_len / inode->i_sb->s_blocksize) + 4;
>
> Can you please explain in the comment what the "+ 4" blocks are?
> I suspect this will not be enough if the xattr is large, it should just
> use one of the standard "transaction size" helper functions to determine
> metadata size.
>
ext4_meta_trans_blocks() will gives the metadata size, which also
account for block group bitmap and block group descriptor blocks,
superblock, inode block.
also here
> @@ -1059,10 +1352,23 @@ ext4_xattr_set(struct inode *inode, int
> const void *value, size_t value_len, int flags)
> {
> handle_t *handle;
> + int buffer_credits;
> int error, retries = 0;
> + buffer_credits = EXT4_DATA_TRANS_BLOCKS(inode->i_sb);
> + if ((value_len >= EXT4_XATTR_MIN_LARGE_EA_SIZE) &&
> + (EXT4_SB(inode->i_sb)->s_es->s_feature_incompat &
> + EXT4_FEATURE_INCOMPAT_EA_INODE)) {
> + /* For new inode */
> + buffer_credits += > > >EXT4_SINGLEDATA_TRANS_BLOCKS(inode->i_sb) +3;
> +
> + /* For the blocks to be written in the EA inode */
> + buffer_credits += (value_len + inode->i_sb->s_blocksize - 1) /
> + inode->i_sb->s_blocksize;
> + }
> +
> retry:
> - handle = ext4_journal_start(inode, > EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
> + handle = ext4_journal_start(inode, buffer_credits);
> if (IS_ERR(handle)) {
> error = PTR_ERR(handle);
> } else {
the credits calculation could be replaced with something like this:
nrblocks = (value_len + inode->i_sb->s_blocksize - 1) / inode->i_sb->s_blocksize;
buffer_credits = ext4_meta_trans_blocks(inode, nrblocks, 1) + nrblocks;
BTW, I think we should update the xttars credits micro in ext4_jbd2.h,
that is based on 1 xattr block assumption...
/* Extended attribute operations touch at most two data buffers,
* two bitmap buffers, and two group summaries, in addition to the inode
* and the superblock, which are already accounted for. */
#define EXT4_XATTR_TRANS_BLOCKS 6U
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
