All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joseph Qi <joseph.qi@huawei.com>
To: ocfs2-devel@oss.oracle.com
Subject: [Ocfs2-devel] [PATCH v2] ocfs2: should not use le32_add_cpu to set ocfs2_dinode i_flags
Date: Sun, 2 Jun 2013 09:32:04 +0800	[thread overview]
Message-ID: <51AAA094.9050508@huawei.com> (raw)
In-Reply-To: <51AA065A.4040400@oracle.com>

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
> 
> .
> 

      reply	other threads:[~2013-06-02  1:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51AAA094.9050508@huawei.com \
    --to=joseph.qi@huawei.com \
    --cc=ocfs2-devel@oss.oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.