New: ext4_inode_inode->i_flags modification is racy

Bug 15792 [email protected] 1 participant 10 messages Apr 15, 2010 – Jul 6, 2010

Share & tools

[email protected]

author

2010-04-15 22:13 UTC #0 lore raw

https://bugzilla.kernel.org/show_bug.cgi?id=15792

           Summary: ext4_inode_inode->i_flags modification is racy
           Product: File System
           Version: 2.5
    Kernel Version: 2.6.33-rc1
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: ext4
        AssignedTo: [email protected]
        ReportedBy: [email protected]
        Regression: No

Created an attachment (id=26025)
--> (https://bugzilla.kernel.org/attachment.cgi?id=26025)
dmesg

I've got an OOPS on 8'cores host under heavy IO load

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[email protected]

author

2010-04-15 22:17 UTC #1 lore raw

[Bug 15792] ext4_inode_inode->i_flags modification is racy

https://bugzilla.kernel.org/show_bug.cgi?id=15792

--- Comment #1 from Dmitry Monakhov <[email protected]> 2010-04-15 22:17:54 --- Created an attachment (id=26026)
--> (https://bugzilla.kernel.org/attachment.cgi?id=26026)
xfstest testcase patch

This is trivial but proven to be useful fsstress based test-case. i'm run it like follows
while true; do ./check 227 || break ; done

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[email protected]

author

2010-04-15 22:32 UTC #2 lore raw

[Bug 15792] ext4_inode_inode->i_flags modification is racy

In reply to [email protected]

https://bugzilla.kernel.org/show_bug.cgi?id=15792

--- Comment #2 from Dmitry Monakhov <[email protected]> 2010-04-15 22:31:55 --- The oops happens because non tested branch was triggered. ./fs/ext4/extents.c

3477:    if (unlikely(EXT4_I(inode)->i_flags & EXT4_EOFBLOCKS_FL)) {
                if (unlikely(!eh->eh_entries)) {
                        EXT4_ERROR_INODE(inode,
                                         "eh->eh_entries == 0 ee_block %d",
                                         ex->ee_block);
### OOPS here because ex == NULL ^^^^^^^^^^^^^^^^^^^^^^^^
                        err = -EIO;
                        goto out2;
                }

Bug was introduced by following commit:
commit 273df556b6ee2065bfe96edab5888d3dc9b108d8

Author: Frank Mayhar <[email protected]>
Date:   Tue Mar 2 11:46:09 2010 -0500

And in fact it is rather trivial to fix.
But most interesting question what the hell we are doing on that error path? inode has EXT4_EOFBLOCKS_FL flag enabled but eh->eh_entries, and in fact after adding more debug information i've found that inode is simply blockless. i_blocks == 0, i_size == 0.

I've collected per-inode i_flag modification history (see an attachment)

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[email protected]

author

2010-04-15 22:39 UTC #3 lore raw

[Bug 15792] ext4_inode_inode->i_flags modification is racy

In reply to [email protected]

https://bugzilla.kernel.org/show_bug.cgi?id=15792

--- Comment #3 from Dmitry Monakhov <[email protected]> 2010-04-15 22:39:15 --- Created an attachment (id=26027)
--> (https://bugzilla.kernel.org/attachment.cgi?id=26027)
i_flags modification history

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[email protected]

author

2010-04-15 22:40 UTC #4 lore raw

[Bug 15792] ext4_inode_inode->i_flags modification is racy

In reply to [email protected]

https://bugzilla.kernel.org/show_bug.cgi?id=15792

Dmitry Monakhov <[email protected]> changed:

What |Removed |Added
----------------------------------------------------------------------------

  Attachment #26027|application/octet-stream    |text/plain
          mime type|                            |

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[email protected]

author

2010-04-15 22:52 UTC #5 lore raw

[Bug 15792] ext4_inode_inode->i_flags modification is racy

In reply to [email protected]

https://bugzilla.kernel.org/show_bug.cgi?id=15792

--- Comment #4 from Dmitry Monakhov <[email protected]> 2010-04-15 22:52:22 --- Most interesting part is last lines

[61] [2]   truncate clr s:737987 d:737987 b:104 fl:80000 
[62] [0]   clr ext4_inode_blocks_set b:104 fl:480000 bit:40000 
[63] [2]   trunc_ext begin s:737987 d:737987 b:104 fl:480000 ml:1

I.E.
CPU2: is doing EXT4_I(inode)->i_flags &= ~EXT4_EOFBLOCKS_FL
CPU0: is doung ei->i_flags &= ~EXT4_HUGE_FILE_FL
CPU2: Wow EXT4_EOFBLOCKS_FL appear again due to race with cpu0. So even if truncate holds i_mutex it is possible to modify i_flags. Seems that we have to modify i_flags via anomic bits operations.

A fix is almost ready. Currently i'm testing it.

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[email protected]

author

2010-04-15 22:54 UTC #6 lore raw

[Bug 15792] ext4_inode_info->i_flags modification is racy

In reply to [email protected]

https://bugzilla.kernel.org/show_bug.cgi?id=15792

Dmitry Monakhov <[email protected]> changed:

What |Removed |Added
----------------------------------------------------------------------------

            Summary|ext4_inode_inode->i_flags   |ext4_inode_info->i_flags
                   |modification is racy        |modification is racy

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[email protected]

author

2010-04-19 03:43 UTC #7 lore raw

[Bug 15792] ext4_inode_info->i_flags modification is racy

In reply to [email protected]

https://bugzilla.kernel.org/show_bug.cgi?id=15792

--- Comment #5 from Dmitry Monakhov <[email protected]> 2010-04-19 03:42:20 --- Ohh.. one more thing, which is not actually related with the name of the bug, But this peace of code was already mention here. Let's just document this here.

fs/ext4/extents.c
3288:int ext4_ext_get_blocks(...)
{
....
3477: if (unlikely(EXT4_I(inode)->i_flags & EXT4_EOFBLOCKS_FL)) {
3478: if (unlikely(!eh->eh_entries)) {
3479: EXT4_ERROR_INODE(inode,
3480: "eh->eh_entries == 0 ee_block %d",
3481: ex->ee_block);
3482: err = -EIO;
3483: goto out2;

3484:           }
3485:           last_ex = EXT_LAST_EXTENT(eh);
3486:           if (iblock + ar.len > le32_to_cpu(last_ex->ee_block)

3487: + ext4_ext_get_actual_len(last_ex))
##### ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ##### If depth > 0 then last_ex may not be the latest extent of the file. ##### because "eh" is just a one of leafs(writing at the middle of a file) ##### We have to find latest extent by traversing a whole path again but ##### always chose the latest eh,ex.
3488: EXT4_I(inode)->i_flags &= ~EXT4_EOFBLOCKS_FL;
for (
3489: }

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[email protected]

author

2010-04-19 14:38 UTC #8 lore raw

[Bug 15792] ext4_inode_info->i_flags modification is racy

In reply to [email protected]

https://bugzilla.kernel.org/show_bug.cgi?id=15792

--- Comment #6 from Dmitry Monakhov <[email protected]> 2010-04-19 14:38:10 --- Proposed patches
http://patchwork.ozlabs.org/patch/50468/
http://patchwork.ozlabs.org/patch/50469/

My hosts survived reasonable amount of time with these patches. If everybody agree with the first patch, I'll prepare similar one for ext3.

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[email protected]

author

2010-07-06 07:15 UTC #9 lore raw

[Bug 15792] ext4_inode_info->i_flags modification is racy

In reply to [email protected]

https://bugzilla.kernel.org/show_bug.cgi?id=15792

Dmitry Monakhov <[email protected]> changed:

What |Removed |Added
----------------------------------------------------------------------------

             Status|NEW                         |RESOLVED
         Resolution|                            |CODE_FIX

Signature · 3 lines

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.