ocfs2: xattr: fix inlined xattr reflink
PATCH 1/2 ocfs2: xattr: fix inlined xattr reflink
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.
--- fs/ocfs2/xattr.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
fs/ocfs2/xattr.c +10 -0
@@ -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 ))); + }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);
Signature · 1 line
1.7.9.5
PATCH 2/2 ocfs2: xattr: remove useless free space checking
free space checking will be done in ocfs2_xattr_ibody_init(). So remove here.
--- fs/ocfs2/xattr.c | 7 ------- 1 file changed, 7 deletions(-)
fs/ocfs2/xattr.c +0 -7
@@ -2759,13 +2759,6 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,down_write (&oi->ip_alloc_sem);if (!(oi->ip_dyn_features &OCFS2_INLINE_XATTR_FL )) { -if (!ocfs2_xattr_has_space_inline (inode, di)) { - ret = -ENOSPC ; - goto out; - } - } - -if (!(oi->ip_dyn_features &OCFS2_INLINE_XATTR_FL )) { ret =ocfs2_xattr_ibody_init (inode, xs->inode_bh, ctxt);if (ret) {if (ret != -ENOSPC )
Signature · 1 line
1.7.9.5
PATCH 2/2 ocfs2: xattr: remove useless free space checking
Quoted 4 lines · Junxiao Bi
free space checking will be done in
ocfs2_xattr_ibody_init(). So remove here.Signed-off-by Junxiao Bi <[email protected]>
Quoted 22 lines
--- fs/ocfs2/xattr.c | 7 ------- 1 file changed, 7 deletions(-)fs/ocfs2/xattr.c +0
-7@@ -2759,13 +2759,6 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,down_write (&oi->ip_alloc_sem);if (!(oi->ip_dyn_features &OCFS2_INLINE_XATTR_FL )) { -if (!ocfs2_xattr_has_space_inline (inode, di)) { - ret = -ENOSPC ; - goto out; - } - } - -if (!(oi->ip_dyn_features &OCFS2_INLINE_XATTR_FL )) { ret =ocfs2_xattr_ibody_init (inode, xs->inode_bh, ctxt);if (ret) {if (ret != -ENOSPC )
Joel Becker
maintainerPATCH 2/2 ocfs2: xattr: remove useless free space checking
Quoted 8 lines · Jeff Liu
Quoted 4 lines · Junxiao Bi
free space checking will be done in
ocfs2_xattr_ibody_init(). So remove here.Signed-off-by Junxiao Bi <[email protected]>Reviewed-by Jie Liu <[email protected]>
Quoted 30 lines
Quoted 13 lines
--- fs/ocfs2/xattr.c | 7 ------- 1 file changed, 7 deletions(-)fs/ocfs2/xattr.c +0
-0@@ -2759,13 +2759,6 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,down_write (&oi->ip_alloc_sem);if (!(oi->ip_dyn_features &OCFS2_INLINE_XATTR_FL )) {fs/ocfs2/xattr.c static int ocfs2_xattr_ibody_set(struct inode *inode, · line 2764
Quoted 10 lines
fs/ocfs2/xattr.c +0
-7-if (!ocfs2_xattr_has_space_inline (inode, di)) { - ret = -ENOSPC ; - goto out; - } - } - -if (!(oi->ip_dyn_features &OCFS2_INLINE_XATTR_FL )) { ret =ocfs2_xattr_ibody_init (inode, xs->inode_bh, ctxt);if (ret) {if (ret != -ENOSPC )_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
Signature · 8 lines
Brain: I shall pollute the water supply with this DNAdefibuliser,
turning everyone into mindless slaves.
Pinky: What about the people who drink bottled water?
Brain: Pinky, people who pay 5 dollars for a bottle of water are
already mindless slaves.
http://www.jlbec.org/
jlbec at evilplan.orgPATCH 1/2 ocfs2: xattr: fix inlined xattr reflink
In reply to Junxiao BiHi Junxiao,
Quoted 20 lines · Junxiao Bi
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 inocfs2_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.Cc <[email protected]>Signed-off-by Junxiao Bi <[email protected]>--- fs/ocfs2/xattr.c | 10 ++++++++++ 1 file changed, 10 insertions(+)fs/ocfs2/xattr.c +0
-0
fs/ocfs2/xattr.c static int ocfs2_reflink_xattr_inline(struct ocfs2_xattr_reflink *args) · line 6507
Quoted 9 lines
fs/ocfs2/xattr.c +6
-0@@ -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))) {
A minor code style adjustment would be better as:
if (!(new_oi->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
!(ocfs2_inode_.....)) {Otherwise this fix looks fine.
fs/ocfs2/xattr.c static int ocfs2_reflink_xattr_inline(struct ocfs2_xattr_reflink *args) · line 6514
Quoted 7 lines
fs/ocfs2/xattr.c +4
-0+struct ocfs2_extent_list *el = &new_di->id2.i_list; +le16_add_cpu (&el->l_count, -(inline_size / +sizeof (struct ocfs2_extent_rec ))); + }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);
Thanks,
-Jeff
PATCH 1/2 ocfs2: xattr: fix inlined xattr reflink
Hi Jeff,
Quoted 34 lines · Jeff Liu
Hi Junxiao,
Quoted 20 lines · Junxiao Bi
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 inocfs2_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.Cc <[email protected]>Signed-off-by Junxiao Bi <[email protected]>--- fs/ocfs2/xattr.c | 10 ++++++++++ 1 file changed, 10 insertions(+)fs/ocfs2/xattr.c +0
-0fs/ocfs2/xattr.c static int ocfs2_reflink_xattr_inline(struct ocfs2_xattr_reflink *args) · line 6507
Quoted 9 lines
fs/ocfs2/xattr.c +6
-0@@ -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))) {A minor code style adjustment would be better as:
Quoted 2 lines
if (!(new_oi->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
!(ocfs2_inode_.....)) {Thanks for review it, will make the change in V2.
Quoted 13 lines
Otherwise this fix looks fine.
fs/ocfs2/xattr.c static int ocfs2_reflink_xattr_inline(struct ocfs2_xattr_reflink *args) · line 6514
Quoted 7 lines
fs/ocfs2/xattr.c +4
-0+struct ocfs2_extent_list *el = &new_di->id2.i_list; +le16_add_cpu (&el->l_count, -(inline_size / +sizeof (struct ocfs2_extent_rec ))); + }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);Thanks,
-Jeff