ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
PATCH 1/1 ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
If we use le32_add_cpu to set ocfs2_dinode i_flags, it may lead to the corresponding flag corrupted. So we should change it to bitwise and/or operation.
--- fs/ocfs2/namei.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
fs/ocfs2/namei.c +3 -3
@@ -522,7 +522,7 @@ static int __ocfs2_mknod_locked(struct inode *dir, fe->i_last_eb_blk =0 ;strcpy (fe->i_signature,OCFS2_INODE_SIGNATURE ); -le32_add_cpu (&fe->i_flags,OCFS2_VALID_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_VALID_FL ); fe->i_atime = fe->i_ctime = fe->i_mtime =cpu_to_le64 (CURRENT_TIME .tv_sec); fe->i_mtime_nsec = fe->i_ctime_nsec = fe->i_atime_nsec =
@@ -2044,7 +2044,7 @@ static int ocfs2_orphan_add(struct ocfs2_super *osb, goto leave; } -le32_add_cpu (&fe->i_flags,OCFS2_ORPHANED_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_ORPHANED_FL );OCFS2_I (inode)->ip_flags &= ~OCFS2_INODE_SKIP_ORPHAN_DIR ;/* Record which orphan dir our inode now resides
@@ -2434,7 +2434,7 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir, } di = (struct ocfs2_dinode *)di_bh->b_data; -le32_add_cpu (&di->i_flags, -OCFS2_ORPHANED_FL ); + di->i_flags &=cpu_to_le32 (~OCFS2_ORPHANED_FL ); di->i_orphaned_slot =0 ;set_nlink (inode,1 );ocfs2_set_links_count (di, inode->i_nlink);
Signature · 1 line
1.7.9.7
PATCH 1/1 ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
Quoted 3 lines · Joseph Qi
If we use
le32_add_cputo setocfs2_dinodei_flags, it may lead to the corresponding flag corrupted. So we should change it to bitwise and/or operation.
I think it should be use bitwise and or operation, because one bit stand for one meaning.
Quoted 37 lines
Signed-off-by Joseph Qi<[email protected]>--- fs/ocfs2/namei.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)fs/ocfs2/namei.c +3
-3@@ -522,7 +522,7 @@ static int __ocfs2_mknod_locked(struct inode *dir, fe->i_last_eb_blk =0 ;strcpy (fe->i_signature,OCFS2_INODE_SIGNATURE ); -le32_add_cpu (&fe->i_flags,OCFS2_VALID_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_VALID_FL ); fe->i_atime = fe->i_ctime = fe->i_mtime =cpu_to_le64 (CURRENT_TIME .tv_sec); fe->i_mtime_nsec = fe->i_ctime_nsec = fe->i_atime_nsec =@@ -2044,7 +2044,7 @@ static int ocfs2_orphan_add(struct ocfs2_super *osb, goto leave; } -le32_add_cpu (&fe->i_flags,OCFS2_ORPHANED_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_ORPHANED_FL );OCFS2_I (inode)->ip_flags&= ~OCFS2_INODE_SKIP_ORPHAN_DIR ;/* Record which orphan dir our inode now resides @@ -2434,7 +2434,7 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir, } di = (struct ocfs2_dinode *)di_bh->b_data; -le32_add_cpu (&di->i_flags, -OCFS2_ORPHANED_FL ); + di->i_flags&=cpu_to_le32 (~OCFS2_ORPHANED_FL ); di->i_orphaned_slot =0 ;set_nlink (inode,1 );ocfs2_set_links_count (di, inode->i_nlink);
PATCH 1/1 ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
In reply to Joseph QiThanks for your patch, Joseph.
Quoted 32 lines · Joseph Qi
If we use
le32_add_cputo setocfs2_dinodei_flags, it may lead to the corresponding flag corrupted. So we should change it to bitwise and/or operation.Signed-off-by Joseph Qi <[email protected]>--- fs/ocfs2/namei.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)fs/ocfs2/namei.c +2
-2@@ -522,7 +522,7 @@ static int __ocfs2_mknod_locked(struct inode *dir, fe->i_last_eb_blk =0 ;strcpy (fe->i_signature,OCFS2_INODE_SIGNATURE ); -le32_add_cpu (&fe->i_flags,OCFS2_VALID_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_VALID_FL ); fe->i_atime = fe->i_ctime = fe->i_mtime =cpu_to_le64 (CURRENT_TIME .tv_sec); fe->i_mtime_nsec = fe->i_ctime_nsec = fe->i_atime_nsec =@@ -2044,7 +2044,7 @@ static int ocfs2_orphan_add(struct ocfs2_super *osb, goto leave; } -le32_add_cpu (&fe->i_flags,OCFS2_ORPHANED_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_ORPHANED_FL );OCFS2_I (inode)->ip_flags &= ~OCFS2_INODE_SKIP_ORPHAN_DIR ;/* Record which orphan dir our inode now resides
fs/ocfs2/namei.c int ocfs2_mv_orphaned_inode_to_new(struct inode *dir, · line 2437
Quoted 5 lines
fs/ocfs2/namei.c +1
-1@@ -2434,7 +2434,7 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir, } di = (struct ocfs2_dinode *)di_bh->b_data; -le32_add_cpu (&di->i_flags, -OCFS2_ORPHANED_FL ); + di->i_flags &=cpu_to_le32 (~OCFS2_ORPHANED_FL );
Hmm? This is wrong. Instead it should be:di->i_flags &= ~cpu_to_le32(OCFS2_ORPHANED_FL);
fs/ocfs2/namei.c int ocfs2_mv_orphaned_inode_to_new(struct inode *dir, · line 2440
Quoted 3 lines
fs/ocfs2/namei.c +0
-0di->i_orphaned_slot =0 ;set_nlink (inode,1 );ocfs2_set_links_count (di, inode->i_nlink);
-Jeff
PATCH 1/1 ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
Quoted 38 lines · Jeff Liu
Thanks for your patch, Joseph.
Quoted 34 lines · Joseph Qi
If we use
le32_add_cputo setocfs2_dinodei_flags, it may lead to the corresponding flag corrupted. So we should change it to bitwise and/or operation.Signed-off-by Joseph Qi<[email protected]>--- fs/ocfs2/namei.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)fs/ocfs2/namei.c +2
-2@@ -522,7 +522,7 @@ static int __ocfs2_mknod_locked(struct inode *dir, fe->i_last_eb_blk =0 ;strcpy (fe->i_signature,OCFS2_INODE_SIGNATURE ); -le32_add_cpu (&fe->i_flags,OCFS2_VALID_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_VALID_FL ); fe->i_atime = fe->i_ctime = fe->i_mtime =cpu_to_le64 (CURRENT_TIME .tv_sec); fe->i_mtime_nsec = fe->i_ctime_nsec = fe->i_atime_nsec =@@ -2044,7 +2044,7 @@ static int ocfs2_orphan_add(struct ocfs2_super *osb, goto leave; } -le32_add_cpu (&fe->i_flags,OCFS2_ORPHANED_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_ORPHANED_FL );OCFS2_I (inode)->ip_flags&= ~OCFS2_INODE_SKIP_ORPHAN_DIR ;/* Record which orphan dir our inode now resides @@ -2434,7 +2434,7 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir, }
Quoted 5 lines
fs/ocfs2/namei.c int ocfs2_mv_orphaned_inode_to_new(struct inode *dir,
Quoted 3 lines
fs/ocfs2/namei.c +1
-1di = (struct ocfs2_dinode *)di_bh->b_data; -le32_add_cpu (&di->i_flags, -OCFS2_ORPHANED_FL ); + di->i_flags&=cpu_to_le32 (~OCFS2_ORPHANED_FL );Hmm? This is wrong. Instead it should be:
di->i_flags&= ~cpu_to_le32(OCFS2_ORPHANED_FL);
in ocfs2_remove_inode function , it has the following code:
di->i_dtime = cpu_to_le64(CURRENT_TIME.tv_sec);
di->i_flags &= cpu_to_le32(~(OCFS2_VALID_FL | OCFS2_ORPHANED_FL));I think at first clear the flag and then convert to the little end.
Quoted 12 lines
fs/ocfs2/namei.c int ocfs2_mv_orphaned_inode_to_new(struct inode *dir, · line 2440
Quoted 3 lines
fs/ocfs2/namei.c +0
-0di->i_orphaned_slot =0 ;set_nlink (inode,1 );ocfs2_set_links_count (di, inode->i_nlink);-Jeff
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
Quoted 47 lines · shencanquan
Quoted 40 lines · Jeff Liu
Thanks for your patch, Joseph.
Quoted 36 lines · Joseph Qi
If we use
le32_add_cputo setocfs2_dinodei_flags, it may lead to the corresponding flag corrupted. So we should change it to bitwise and/or operation.Signed-off-by Joseph Qi<[email protected]>--- fs/ocfs2/namei.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)fs/ocfs2/namei.c +2
-2@@ -522,7 +522,7 @@ static int __ocfs2_mknod_locked(struct inode *dir, fe->i_last_eb_blk =0 ;strcpy (fe->i_signature,OCFS2_INODE_SIGNATURE ); -le32_add_cpu (&fe->i_flags,OCFS2_VALID_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_VALID_FL ); fe->i_atime = fe->i_ctime = fe->i_mtime =cpu_to_le64 (CURRENT_TIME .tv_sec); fe->i_mtime_nsec = fe->i_ctime_nsec = fe->i_atime_nsec =@@ -2044,7 +2044,7 @@ static int ocfs2_orphan_add(struct ocfs2_super *osb, goto leave; } -le32_add_cpu (&fe->i_flags,OCFS2_ORPHANED_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_ORPHANED_FL );OCFS2_I (inode)->ip_flags&= ~OCFS2_INODE_SKIP_ORPHAN_DIR ;/* Record which orphan dir our inode now residesfs/ocfs2/namei.c +0
-0@@ -2434,7 +2434,7 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir, }Quoted 5 lines
fs/ocfs2/namei.c int ocfs2_mv_orphaned_inode_to_new(struct inode *dir,
Quoted 3 lines
fs/ocfs2/namei.c +1
-1di = (struct ocfs2_dinode *)di_bh->b_data; -le32_add_cpu (&di->i_flags, -OCFS2_ORPHANED_FL ); + di->i_flags&=cpu_to_le32 (~OCFS2_ORPHANED_FL );Hmm? This is wrong. Instead it should be:
di->i_flags&= ~cpu_to_le32(OCFS2_ORPHANED_FL);
Quoted 5 lines
in
ocfs2_remove_inodefunction , it has the following code:di->i_dtime = cpu_to_le64(CURRENT_TIME.tv_sec); di->i_flags &= cpu_to_le32(~(OCFS2_VALID_FL | OCFS2_ORPHANED_FL));I think at first clear the flag and then convert to the little end.
Call cpu_to_xxx(~value) or (~cpu_to_xxx(value)) will be evaluated end up with the same result, but the later manner would looks a bit neater IMO.
Thanks,
-Jeff
PATCH 1/1 ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
You are right. Thanks for your review comments.
I will do the corresponding modification and resend the patch.
Quoted 63 lines · Jeff Liu
Quoted 47 lines · shencanquan
Quoted 40 lines · Jeff Liu
Thanks for your patch, Joseph.
Quoted 36 lines · Joseph Qi
If we use
le32_add_cputo setocfs2_dinodei_flags, it may lead to the corresponding flag corrupted. So we should change it to bitwise and/or operation.Signed-off-by Joseph Qi<[email protected]>--- fs/ocfs2/namei.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)fs/ocfs2/namei.c +2
-2@@ -522,7 +522,7 @@ static int __ocfs2_mknod_locked(struct inode *dir, fe->i_last_eb_blk =0 ;strcpy (fe->i_signature,OCFS2_INODE_SIGNATURE ); -le32_add_cpu (&fe->i_flags,OCFS2_VALID_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_VALID_FL ); fe->i_atime = fe->i_ctime = fe->i_mtime =cpu_to_le64 (CURRENT_TIME .tv_sec); fe->i_mtime_nsec = fe->i_ctime_nsec = fe->i_atime_nsec =@@ -2044,7 +2044,7 @@ static int ocfs2_orphan_add(struct ocfs2_super *osb, goto leave; } -le32_add_cpu (&fe->i_flags,OCFS2_ORPHANED_FL ); + fe->i_flags |=cpu_to_le32 (OCFS2_ORPHANED_FL );OCFS2_I (inode)->ip_flags&= ~OCFS2_INODE_SKIP_ORPHAN_DIR ;/* Record which orphan dir our inode now residesfs/ocfs2/namei.c +0
-0@@ -2434,7 +2434,7 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir, }Quoted 5 lines
fs/ocfs2/namei.c int ocfs2_mv_orphaned_inode_to_new(struct inode *dir,
Quoted 3 lines
fs/ocfs2/namei.c +1
-1di = (struct ocfs2_dinode *)di_bh->b_data; -le32_add_cpu (&di->i_flags, -OCFS2_ORPHANED_FL ); + di->i_flags&=cpu_to_le32 (~OCFS2_ORPHANED_FL );Hmm? This is wrong. Instead it should be:
di->i_flags&= ~cpu_to_le32(OCFS2_ORPHANED_FL);Quoted 5 lines
in
ocfs2_remove_inodefunction , it has the following code:di->i_dtime = cpu_to_le64(CURRENT_TIME.tv_sec); di->i_flags &= cpu_to_le32(~(OCFS2_VALID_FL | OCFS2_ORPHANED_FL));I think at first clear the flag and then convert to the little end.
Call
cpu_to_xxx(~value)or (~cpu_to_xxx(value)) will be evaluated end up with the same result, but the later manner would looks a bit neater IMO.Thanks,
-Jeff.