Hi Andrew,
On 06/28/2013 07:08 AM, Andrew Morton wrote:
> On Fri, 21 Jun 2013 09:27:24 +0800 Junxiao Bi <junxiao...@oracle.com> wrote:
>
>> Inlined xattr shared free space of inode block with inlined data
>> or data extent record, so the size of the later two should be
>> adjusted when inlined xattr is enabled. See ocfs2_xattr_ibody_init().
>> But this isn't done well when reflink. For inode with inlined data,
>> its max inlined data size is adjusted in ocfs2_duplicate_inline_data(),
>> no problem. But for inode with data extent record, its record count isn't
>> adjusted. Fix it, or data extent record and inlined xattr may overwrite
>> each other, then cause data corruption or xattr failure.
>>
>> One panic caused by this bug in our test environment is the following:
>>
>> kernel BUG at fs/ocfs2/xattr.c:1435!
>>
>> ...
>>
>> @@ -6499,6 +6499,16 @@ static int ocfs2_reflink_xattr_inline(struct
>> ocfs2_xattr_reflink *args)
>> }
>>
>> new_oi = OCFS2_I(args->new_inode);
>> + /*
>> + * Adjust extent record count to reserve space for extended attribute.
>> + * Inline data count had been adjusted in ocfs2_duplicate_inline_data().
>> + */
>> + if (!(new_oi->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>> + !(ocfs2_inode_is_fast_symlink(new_inode))) {
>> + struct ocfs2_extent_list *el = &new_di->id2.i_list;
>> + le16_add_cpu(&el->l_count, -(inline_size /
>> + sizeof(struct ocfs2_extent_rec)));
>> + }
> This is badly screwed up. There is no local variable `new_inode' -
> new_inode() is a global function!
>
> I assume you meant args->new_inode, but this patch clearly wasn't
> tested (or wasn't what you _did_ test).
Sorry, I made a bad mistake, it should be args->new_inode,
I just test the case where the inode is not fast symlink.
I will fix this and test all the case and send again.
Thanks,
Junxiao.
>
> I'll drop it. Please fix, retest, resend.
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
