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.

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

index 04ee1b5..f53471d 100644
@@ -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);

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.

Quoted 4 lines · On Thu, 30 May 2013 12:49:46 +0800 Joseph Qi <[email protected]> wrote:

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.

The usual question: what is the end-user impact of the bug which was just fixed?

For the usual reason: so I and others can decide which kernel versions need the fix.

Hi Andrew,

Sorry for the late response because I just returned from the LinuxCon/Japan.

I'm afraid that Joesph can not follow this thread up in time since today is the weekend in China. So please let me answer your question on behalf of Joesph as a quick response.

Quoted 9 lines · Andrew Morton

Quoted 4 lines · On Thu, 30 May 2013 12:49:46 +0800 Joseph Qi <[email protected]> wrote:

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.

The usual question: what is the end-user impact of the bug which was just fixed?

This patch can be treated as a trivial fix. Yes, i_flags is operated in bitwise context, but those assignments are only happened at the time of initializing the corresponding OCFS2 private on-disk inode info. Hence there is no impact from the end-user's perspective.

However, it's better to replace those three assignments with bitwise operations since they essentially should be. In this way, we can immediately know that the business performed on i_flags are in bitwise rather than increasing or decreasing a count value.

Quoted 2 lines

For the usual reason: so I and others can decide which kernel versions need the fix.

The current fix comments looks too horrible than it would be. Can it make your life easier if I repost this patch with the revised descriptions if Joseph agrees to me?

Thanks,
-Jeff

.