* [Ocfs2-devel] [PATCH v2] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
@ 2013-05-30 4:49 Joseph Qi
2013-05-30 6:48 ` Jeff Liu
2013-05-31 22:25 ` Andrew Morton
0 siblings, 2 replies; 5+ messages in thread
From: Joseph Qi @ 2013-05-30 4:49 UTC (permalink / raw)
To: ocfs2-devel
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 <joseph.qi@huawei.com>
---
fs/ocfs2/namei.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 04ee1b5..f53471d 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -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);
--
1.7.9.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH v2] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
2013-05-30 4:49 [Ocfs2-devel] [PATCH v2] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags Joseph Qi
@ 2013-05-30 6:48 ` Jeff Liu
2013-05-31 22:25 ` Andrew Morton
1 sibling, 0 replies; 5+ messages in thread
From: Jeff Liu @ 2013-05-30 6:48 UTC (permalink / raw)
To: ocfs2-devel
On 05/30/2013 12:49 PM, Joseph Qi 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.
>
> Signed-off-by: Joseph Qi <joseph.qi@huawei.com>
Reviewed-by: Jie Liu <jeff.liu@oracle.com>
> ---
> fs/ocfs2/namei.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
> index 04ee1b5..f53471d 100644
> --- a/fs/ocfs2/namei.c
> +++ b/fs/ocfs2/namei.c
> @@ -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);
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH v2] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
2013-05-30 4:49 [Ocfs2-devel] [PATCH v2] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags Joseph Qi
2013-05-30 6:48 ` Jeff Liu
@ 2013-05-31 22:25 ` Andrew Morton
2013-06-01 14:34 ` Jeff Liu
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2013-05-31 22:25 UTC (permalink / raw)
To: ocfs2-devel
On Thu, 30 May 2013 12:49:46 +0800 Joseph Qi <joseph.qi@huawei.com> 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.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH v2] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
2013-05-31 22:25 ` Andrew Morton
@ 2013-06-01 14:34 ` Jeff Liu
2013-06-02 1:32 ` Joseph Qi
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Liu @ 2013-06-01 14:34 UTC (permalink / raw)
To: ocfs2-devel
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.
On 06/01/2013 06:25 AM, Andrew Morton wrote:
> On Thu, 30 May 2013 12:49:46 +0800 Joseph Qi <joseph.qi@huawei.com> 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.
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH v2] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
2013-06-01 14:34 ` Jeff Liu
@ 2013-06-02 1:32 ` Joseph Qi
0 siblings, 0 replies; 5+ messages in thread
From: Joseph Qi @ 2013-06-02 1:32 UTC (permalink / raw)
To: ocfs2-devel
Hi Jeff & Andrew,
We found this bug when we do analysis for a read-only filesystem bug
that is directly caused by dinode flags mismatch.
Of course Jeff you could fix the comments and make it comprehensible.
On 2013/6/1 22:34, Jeff Liu wrote:
> 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.
>
> On 06/01/2013 06:25 AM, Andrew Morton wrote:
>
>> On Thu, 30 May 2013 12:49:46 +0800 Joseph Qi <joseph.qi@huawei.com> 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.
>
>> 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
>
> .
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-06-02 1:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-30 4:49 [Ocfs2-devel] [PATCH v2] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags Joseph Qi
2013-05-30 6:48 ` Jeff Liu
2013-05-31 22:25 ` Andrew Morton
2013-06-01 14:34 ` Jeff Liu
2013-06-02 1:32 ` Joseph Qi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.