From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junxiao Bi Date: Tue, 02 Jul 2013 09:08:36 +0800 Subject: [Ocfs2-devel] [PATCH V3] ocfs2: xattr: fix inlined xattr reflink In-Reply-To: <20130701225746.GC965@localhost> References: <1372388183-6378-1-git-send-email-junxiao.bi@oracle.com> <20130701225746.GC965@localhost> Message-ID: <51D22814.7050500@oracle.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ocfs2-devel@oss.oracle.com Hi Joel, On 07/02/2013 06:57 AM, Joel Becker wrote: > On Fri, Jun 28, 2013 at 10:56:23AM +0800, Junxiao Bi 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! >> invalid opcode: 0000 [#1] SMP >> CPU 0 >> Modules linked in: ocfs2 jbd2 ocfs2_dlmfs ocfs2_stack_o2cb ocfs2_dlm >> ocfs2_nodemanager ocfs2_stackglue configfs sunrpc parport_pc lp parport >> snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device >> snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd soundcore snd_page_alloc >> pcspkr xen_netfront dm_snapshot dm_zero dm_mirror dm_region_hash dm_log >> dm_mod xen_blkfront ext3 jbd mbcache [last unloaded: configfs] >> >> Pid: 10871, comm: multi_reflink_t Not tainted 2.6.39-300.17.1.el5uek #1 >> RIP: e030:[] [] >> ocfs2_xa_offset_pointer+0x17/0x20 [ocfs2] >> RSP: e02b:ffff88007a587948 EFLAGS: 00010283 >> RAX: 0000000000000000 RBX: 0000000000000010 RCX: 00000000000051e4 >> RDX: ffff880057092060 RSI: 0000000000000f80 RDI: ffff88007a587a68 >> RBP: ffff88007a587948 R08: 00000000000062f4 R09: 0000000000000000 >> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000010 >> R13: ffff88007a587a68 R14: 0000000000000001 R15: ffff88007a587c68 >> FS: 00007fccff7f06e0(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000 >> CS: e033 DS: 0000 ES: 0000 CR0: 000000008005003b >> CR2: 00000000015cf000 CR3: 000000007aa76000 CR4: 0000000000000660 >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 >> Process multi_reflink_t (pid: 10871, threadinfo ffff88007a586000, task >> ffff88007911c380) >> Stack: >> ffff88007a5879a8 ffffffffa04d0a60 ffff88007a587998 ffffffffa04cd514 >> ffff88007a587c38 ffffffffa04cc125 000000007a587998 ffff88007a587a68 >> ffff88007a587c68 0000000000000000 00000000000003c2 ffff88007a587c38 >> Call Trace: >> [] ocfs2_xa_reuse_entry+0x60/0x280 [ocfs2] >> [] ? ocfs2_xa_block_check_space+0x84/0xa0 [ocfs2] >> [] ? namevalue_size_xi+0x15/0x20 [ocfs2] >> [] ocfs2_xa_prepare_entry+0x17e/0x2a0 [ocfs2] >> [] ocfs2_xa_set+0xcc/0x250 [ocfs2] >> [] ocfs2_xattr_ibody_set+0x98/0x230 [ocfs2] >> [] ? kmem_cache_alloc+0xab/0x190 >> [] __ocfs2_xattr_set_handle+0x4f/0x700 [ocfs2] >> [] ? jbd2_journal_start+0x13/0x20 [jbd2] >> [] ocfs2_xattr_set+0x6c6/0x890 [ocfs2] >> [] ocfs2_xattr_user_set+0x46/0x50 [ocfs2] >> [] generic_setxattr+0x70/0x90 >> [] __vfs_setxattr_noperm+0x80/0x1a0 >> [] vfs_setxattr+0xa9/0xb0 >> [] setxattr+0xc3/0x120 >> [] ? finish_task_switch+0x70/0xe0 >> [] ? __schedule+0x364/0x6d0 >> [] sys_fsetxattr+0xa8/0xd0 >> [] system_call_fastpath+0x16/0x1b >> Code: 0f 1f 40 00 eb ae 0f 0b eb fe 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 >> 66 66 66 66 90 39 77 10 7e 09 48 8b 47 28 ff 50 10 c9 c3 <0f> 0b eb fe 0f 1f >> 44 00 00 55 48 89 e5 53 48 83 ec 08 66 66 66 >> RIP [] ocfs2_xa_offset_pointer+0x17/0x20 [ocfs2] >> RSP >> ---[ end trace 33461d2444bd1e5e ]--- >> >> Cc: >> Signed-off-by: Junxiao Bi >> Reviewed-by: Jie Liu >> --- >> fs/ocfs2/xattr.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c >> index 2e3ea30..5b8d944 100644 >> --- a/fs/ocfs2/xattr.c >> +++ b/fs/ocfs2/xattr.c >> @@ -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(args->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))); >> + } > Why are you referencing the inode before taking the lock? I think we didn't need take the lock here because this is in reflink, it's a new inode. It can not be access from other process or node at that time. Thanks, Junxiao. > > Joel > >> spin_lock(&new_oi->ip_lock); >> new_oi->ip_dyn_features |= OCFS2_HAS_XATTR_FL | OCFS2_INLINE_XATTR_FL; >> new_di->i_dyn_features = cpu_to_le16(new_oi->ip_dyn_features); >> -- >> 1.7.9.5 >> >> >> _______________________________________________ >> Ocfs2-devel mailing list >> Ocfs2-devel at oss.oracle.com >> https://oss.oracle.com/mailman/listinfo/ocfs2-devel