* [Ocfs2-devel] [PATCH] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
@ 2013-05-29 2:42 Joseph Qi
2013-05-29 7:02 ` shencanquan
2013-05-29 7:57 ` Jeff Liu
0 siblings, 2 replies; 6+ messages in thread
From: Joseph Qi @ 2013-05-29 2:42 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..3a5269a 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] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
2013-05-29 2:42 [Ocfs2-devel] [PATCH] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags Joseph Qi
@ 2013-05-29 7:02 ` shencanquan
2013-05-29 7:57 ` Jeff Liu
1 sibling, 0 replies; 6+ messages in thread
From: shencanquan @ 2013-05-29 7:02 UTC (permalink / raw)
To: ocfs2-devel
On 2013/5/29 10:42, 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.
I think it should be use bitwise and or operation, because one bit stand
for one meaning.
>
> 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..3a5269a 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] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
2013-05-29 2:42 [Ocfs2-devel] [PATCH] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags Joseph Qi
2013-05-29 7:02 ` shencanquan
@ 2013-05-29 7:57 ` Jeff Liu
2013-05-29 8:12 ` shencanquan
1 sibling, 1 reply; 6+ messages in thread
From: Jeff Liu @ 2013-05-29 7:57 UTC (permalink / raw)
To: ocfs2-devel
Thanks for your patch, Joseph.
On 05/29/2013 10:42 AM, 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>
> ---
> 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..3a5269a 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);
Hmm? This is wrong. Instead it should be:
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);
-Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
2013-05-29 7:57 ` Jeff Liu
@ 2013-05-29 8:12 ` shencanquan
2013-05-29 10:17 ` Jeff Liu
0 siblings, 1 reply; 6+ messages in thread
From: shencanquan @ 2013-05-29 8:12 UTC (permalink / raw)
To: ocfs2-devel
On 2013/5/29 15:57, Jeff Liu wrote:
> Thanks for your patch, Joseph.
>
> On 05/29/2013 10:42 AM, 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>
>> ---
>> 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..3a5269a 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);
> 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.
>> di->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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
2013-05-29 8:12 ` shencanquan
@ 2013-05-29 10:17 ` Jeff Liu
2013-05-30 4:40 ` Joseph Qi
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Liu @ 2013-05-29 10:17 UTC (permalink / raw)
To: ocfs2-devel
On 05/29/2013 04:12 PM, shencanquan wrote:
> On 2013/5/29 15:57, Jeff Liu wrote:
>> Thanks for your patch, Joseph.
>>
>> On 05/29/2013 10:42 AM, 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>
>>> ---
>>> 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..3a5269a 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);
>> 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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
2013-05-29 10:17 ` Jeff Liu
@ 2013-05-30 4:40 ` Joseph Qi
0 siblings, 0 replies; 6+ messages in thread
From: Joseph Qi @ 2013-05-30 4:40 UTC (permalink / raw)
To: ocfs2-devel
You are right. Thanks for your review comments.
I will do the corresponding modification and resend the patch.
On 2013/5/29 18:17, Jeff Liu wrote:
> On 05/29/2013 04:12 PM, shencanquan wrote:
>
>> On 2013/5/29 15:57, Jeff Liu wrote:
>>> Thanks for your patch, Joseph.
>>>
>>> On 05/29/2013 10:42 AM, 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>
>>>> ---
>>>> 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..3a5269a 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);
>>> 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.
>
> 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
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-30 4:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-29 2:42 [Ocfs2-devel] [PATCH] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags Joseph Qi
2013-05-29 7:02 ` shencanquan
2013-05-29 7:57 ` Jeff Liu
2013-05-29 8:12 ` shencanquan
2013-05-29 10:17 ` Jeff Liu
2013-05-30 4:40 ` 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.