* Fwd: Data race in __inode_add_bytes [not found] <CAAeHK+womwjgo4UgErWzDC9XD7BwUB1TkaFLo0eC7jF0ftV6gg@mail.gmail.com> @ 2015-08-31 19:35 ` Andrey Konovalov 2015-09-01 9:21 ` Dmitry Vyukov [not found] ` <20150925090016.GB2806@quack.suse.cz> 1 sibling, 1 reply; 4+ messages in thread From: Andrey Konovalov @ 2015-08-31 19:35 UTC (permalink / raw) To: Alexander Viro, linux-fsdevel, Theodore Ts'o, Andreas Dilger, linux-ext4, Aneesh Kumar K.V, linux-kernel Cc: Dmitry Vyukov, Alexander Potapenko, Kostya Serebryany Hi! We are working on a dynamic data race detector for the Linux kernel, KernelThreadSanitizer (ktsan): https://github.com/google/ktsan/wiki We got a report while running ktsan on 4.2: ================================================================== ThreadSanitizer: data-race in __inode_add_bytes Write of size 8 by thread T210 (K740): [<ffffffff81266435>] __inode_add_bytes+0x55/0xd0 fs/stat.c:451 [<ffffffff812f90c0>] inode_claim_rsv_space+0x60/0xa0 fs/quota/dquot.c:1557 [<ffffffff812f9f7b>] dquot_claim_space_nodirty+0x3b/0x280 fs/quota/dquot.c:1721 [< inlined >] ext4_da_update_reserve_space+0x13b/0x2c0 dquot_claim_block include/linux/quotaops.h:345 [<ffffffff81335dab>] ext4_da_update_reserve_space+0x13b/0x2c0 fs/ext4/inode.c:350 [<ffffffff81384cf0>] ext4_ext_map_blocks+0x1570/0x1a30 fs/ext4/extents.c:4597 [<ffffffff8133610a>] ext4_map_blocks+0x1da/0x7b0 fs/ext4/inode.c:592 [< inlined >] ext4_writepages+0x976/0x1480 mpage_map_one_extent fs/ext4/inode.c:2109 [< inlined >] ext4_writepages+0x976/0x1480 mpage_map_and_submit_extent fs/ext4/inode.c:2165 [<ffffffff8133b7b6>] ext4_writepages+0x976/0x1480 fs/ext4/inode.c:2508 [<ffffffff811dbd23>] do_writepages+0x53/0x80 mm/page-writeback.c:2332 [<ffffffff812a76bf>] __writeback_single_inode+0x7f/0x530 fs/fs-writeback.c:1259 (discriminator 3) [<ffffffff812a7fd4>] writeback_sb_inodes+0x464/0x690 fs/fs-writeback.c:1516 [<ffffffff812a82c1>] __writeback_inodes_wb+0xc1/0x100 fs/fs-writeback.c:1562 [<ffffffff812a86ae>] wb_writeback+0x3ae/0x450 fs/fs-writeback.c:1666 [< inlined >] wb_workfn+0x203/0x780 wb_do_writeback fs/fs-writeback.c:1801 [<ffffffff812a91c3>] wb_workfn+0x203/0x780 fs/fs-writeback.c:1852 [<ffffffff810b06ce>] process_one_work+0x28e/0x710 kernel/workqueue.c:2036 [<ffffffff810b1299>] worker_thread+0xb9/0x750 kernel/workqueue.c:2170 [<ffffffff810b9c61>] kthread+0x161/0x180 kernel/kthread.c:209 [<ffffffff81eb0a1f>] ret_from_fork+0x3f/0x70 arch/x86/entry/entry_64.S:526 Previous read of size 8 by thread T512 (K7200): [< inlined >] ext4_mark_iloc_dirty+0x454/0xe20 ext4_inode_blocks_set fs/ext4/inode.c:4272 [< inlined >] ext4_mark_iloc_dirty+0x454/0xe20 ext4_do_update_inode fs/ext4/inode.c:4430 [<ffffffff8133a014>] ext4_mark_iloc_dirty+0x454/0xe20 fs/ext4/inode.c:4937 [<ffffffff8133ab8b>] ext4_mark_inode_dirty+0xdb/0x390 fs/ext4/inode.c:5053 [<ffffffff8133f929>] ext4_dirty_inode+0x59/0x80 fs/ext4/inode.c:5085 [<ffffffff812a7319>] __mark_inode_dirty+0x2c9/0x5f0 fs/fs-writeback.c:2015 [<ffffffff8128a58e>] generic_update_time+0xbe/0x150 fs/inode.c:1566 [< inlined >] file_update_time+0x112/0x1b0 update_time fs/inode.c:1582 [<ffffffff812890f2>] file_update_time+0x112/0x1b0 fs/inode.c:1785 [<ffffffff811cb175>] __generic_file_write_iter+0x105/0x2e0 mm/filemap.c:2570 [<ffffffff8132c3a4>] ext4_file_write_iter+0x254/0x740 fs/ext4/file.c:170 [< inlined >] __vfs_write+0x19c/0x1e0 new_sync_write fs/read_write.c:478 [<ffffffff8125d48c>] __vfs_write+0x19c/0x1e0 fs/read_write.c:491 [<ffffffff8125dde6>] vfs_write+0xf6/0x2a0 fs/read_write.c:538 [< inlined >] SyS_write+0x6b/0xd0 SYSC_write fs/read_write.c:585 [<ffffffff8125f37b>] SyS_write+0x6b/0xd0 fs/read_write.c:577 [<ffffffff81eb062e>] entry_SYSCALL_64_fastpath+0x12/0x71 arch/x86/entry/entry_64.S:186 ================================================================== The 'inode->i_blocks' field is updated in one thread, while being read and used in another. This can probably be fixed with a few READ_ONCE/WRITE_ONCE or by taking inode->i_lock in ext4_inode_blocks_set. Thanks. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Data race in __inode_add_bytes 2015-08-31 19:35 ` Fwd: Data race in __inode_add_bytes Andrey Konovalov @ 2015-09-01 9:21 ` Dmitry Vyukov 0 siblings, 0 replies; 4+ messages in thread From: Dmitry Vyukov @ 2015-09-01 9:21 UTC (permalink / raw) To: Andrey Konovalov Cc: Alexander Viro, linux-fsdevel, Theodore Ts'o, Andreas Dilger, linux-ext4, Aneesh Kumar K.V, LKML, Alexander Potapenko, Kostya Serebryany On Mon, Aug 31, 2015 at 9:35 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > Hi! > > We are working on a dynamic data race detector for the Linux kernel, > KernelThreadSanitizer (ktsan): > https://github.com/google/ktsan/wiki > > We got a report while running ktsan on 4.2: > > ================================================================== > ThreadSanitizer: data-race in __inode_add_bytes > > Write of size 8 by thread T210 (K740): > [<ffffffff81266435>] __inode_add_bytes+0x55/0xd0 fs/stat.c:451 > [<ffffffff812f90c0>] inode_claim_rsv_space+0x60/0xa0 fs/quota/dquot.c:1557 > [<ffffffff812f9f7b>] dquot_claim_space_nodirty+0x3b/0x280 fs/quota/dquot.c:1721 > [< inlined >] ext4_da_update_reserve_space+0x13b/0x2c0 > dquot_claim_block include/linux/quotaops.h:345 > [<ffffffff81335dab>] ext4_da_update_reserve_space+0x13b/0x2c0 > fs/ext4/inode.c:350 > [<ffffffff81384cf0>] ext4_ext_map_blocks+0x1570/0x1a30 fs/ext4/extents.c:4597 > [<ffffffff8133610a>] ext4_map_blocks+0x1da/0x7b0 fs/ext4/inode.c:592 > [< inlined >] ext4_writepages+0x976/0x1480 > mpage_map_one_extent fs/ext4/inode.c:2109 > [< inlined >] ext4_writepages+0x976/0x1480 > mpage_map_and_submit_extent fs/ext4/inode.c:2165 > [<ffffffff8133b7b6>] ext4_writepages+0x976/0x1480 fs/ext4/inode.c:2508 > [<ffffffff811dbd23>] do_writepages+0x53/0x80 mm/page-writeback.c:2332 > [<ffffffff812a76bf>] __writeback_single_inode+0x7f/0x530 > fs/fs-writeback.c:1259 (discriminator 3) > [<ffffffff812a7fd4>] writeback_sb_inodes+0x464/0x690 fs/fs-writeback.c:1516 > [<ffffffff812a82c1>] __writeback_inodes_wb+0xc1/0x100 fs/fs-writeback.c:1562 > [<ffffffff812a86ae>] wb_writeback+0x3ae/0x450 fs/fs-writeback.c:1666 > [< inlined >] wb_workfn+0x203/0x780 wb_do_writeback > fs/fs-writeback.c:1801 > [<ffffffff812a91c3>] wb_workfn+0x203/0x780 fs/fs-writeback.c:1852 > [<ffffffff810b06ce>] process_one_work+0x28e/0x710 kernel/workqueue.c:2036 > [<ffffffff810b1299>] worker_thread+0xb9/0x750 kernel/workqueue.c:2170 > [<ffffffff810b9c61>] kthread+0x161/0x180 kernel/kthread.c:209 > [<ffffffff81eb0a1f>] ret_from_fork+0x3f/0x70 arch/x86/entry/entry_64.S:526 > > Previous read of size 8 by thread T512 (K7200): > [< inlined >] ext4_mark_iloc_dirty+0x454/0xe20 > ext4_inode_blocks_set fs/ext4/inode.c:4272 > [< inlined >] ext4_mark_iloc_dirty+0x454/0xe20 > ext4_do_update_inode fs/ext4/inode.c:4430 > [<ffffffff8133a014>] ext4_mark_iloc_dirty+0x454/0xe20 fs/ext4/inode.c:4937 > [<ffffffff8133ab8b>] ext4_mark_inode_dirty+0xdb/0x390 fs/ext4/inode.c:5053 > [<ffffffff8133f929>] ext4_dirty_inode+0x59/0x80 fs/ext4/inode.c:5085 > [<ffffffff812a7319>] __mark_inode_dirty+0x2c9/0x5f0 fs/fs-writeback.c:2015 > [<ffffffff8128a58e>] generic_update_time+0xbe/0x150 fs/inode.c:1566 > [< inlined >] file_update_time+0x112/0x1b0 update_time fs/inode.c:1582 > [<ffffffff812890f2>] file_update_time+0x112/0x1b0 fs/inode.c:1785 > [<ffffffff811cb175>] __generic_file_write_iter+0x105/0x2e0 mm/filemap.c:2570 > [<ffffffff8132c3a4>] ext4_file_write_iter+0x254/0x740 fs/ext4/file.c:170 > [< inlined >] __vfs_write+0x19c/0x1e0 new_sync_write fs/read_write.c:478 > [<ffffffff8125d48c>] __vfs_write+0x19c/0x1e0 fs/read_write.c:491 > [<ffffffff8125dde6>] vfs_write+0xf6/0x2a0 fs/read_write.c:538 > [< inlined >] SyS_write+0x6b/0xd0 SYSC_write fs/read_write.c:585 > [<ffffffff8125f37b>] SyS_write+0x6b/0xd0 fs/read_write.c:577 > [<ffffffff81eb062e>] entry_SYSCALL_64_fastpath+0x12/0x71 > arch/x86/entry/entry_64.S:186 > ================================================================== > > The 'inode->i_blocks' field is updated in one thread, while being read > and used in another. > > This can probably be fixed with a few READ_ONCE/WRITE_ONCE or by > taking inode->i_lock in ext4_inode_blocks_set. READ_ONCE/WRITE_ONCE won't do since i_blocks is u64. ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <20150925090016.GB2806@quack.suse.cz>]
[parent not found: <CACT4Y+bWXRnNwmv+p2wKQY+c9e0VOsCqSkUiLa_1s54gCV7HJg@mail.gmail.com>]
* Re: Data race in __inode_add_bytes [not found] ` <CACT4Y+bWXRnNwmv+p2wKQY+c9e0VOsCqSkUiLa_1s54gCV7HJg@mail.gmail.com> @ 2015-09-25 9:51 ` Dmitry Vyukov 2015-09-25 11:52 ` Jan Kara 0 siblings, 1 reply; 4+ messages in thread From: Dmitry Vyukov @ 2015-09-25 9:51 UTC (permalink / raw) To: Jan Kara Cc: Andrey Konovalov, Alexander Viro, linux-fsdevel, Theodore Ts'o, Andreas Dilger, linux-ext4, Aneesh Kumar K.V, Alexander Potapenko, LKML, ktsan Re-sending with correct mailing list address. On Fri, Sep 25, 2015 at 11:36 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > Looking at __inode_add_bytes/ext4_inode_blocks_set I see much more > ways to screw things up. > For example, __inode_add_bytes: > > void __inode_add_bytes(struct inode *inode, loff_t bytes) > { > inode->i_blocks += bytes >> 9; > bytes &= 511; > inode->i_bytes += bytes; > if (inode->i_bytes >= 512) { > inode->i_blocks++; > inode->i_bytes -= 512; > } > } > > can be compiled effectively as: > > void __inode_add_bytes(struct inode *inode, loff_t bytes) > { > inode->i_blocks += bytes >> 9 + 1; > bytes = inode->i_bytes + (bytes & 511); > if (bytes < 512) > inode->i_blocks--; > inode->i_bytes = bytes & 511; > } > > Which will produce invalid results on any bitness with any file size. > > Or, for example, ext4_inode_blocks_set can read inode->i_blocks > several times and then also produce invalid results (lose high part or > something). > > Since this is invalid C code, there is basically infinite number of > ways how this can screw up by compiler and/or hardware. > > > > > > On Fri, Sep 25, 2015 at 11:00 AM, Jan Kara <jack@suse.cz> wrote: >> On Mon 31-08-15 21:33:46, Andrey Konovalov wrote: >>> Hi! >>> >>> We are working on a dynamic data race detector for the Linux kernel, >>> KernelThreadSanitizer (ktsan): >>> https://github.com/google/ktsan/wiki >>> >>> We got a report while running ktsan on 4.2: >>> >>> ================================================================== >>> ThreadSanitizer: data-race in __inode_add_bytes >>> >>> Write of size 8 by thread T210 (K740): >>> [<ffffffff81266435>] __inode_add_bytes+0x55/0xd0 fs/stat.c:451 >>> [<ffffffff812f90c0>] inode_claim_rsv_space+0x60/0xa0 fs/quota/dquot.c:1557 >>> [<ffffffff812f9f7b>] dquot_claim_space_nodirty+0x3b/0x280 fs/quota/dquot.c:1721 >>> [< inlined >] ext4_da_update_reserve_space+0x13b/0x2c0 >>> dquot_claim_block include/linux/quotaops.h:345 >>> [<ffffffff81335dab>] ext4_da_update_reserve_space+0x13b/0x2c0 >>> fs/ext4/inode.c:350 >>> [<ffffffff81384cf0>] ext4_ext_map_blocks+0x1570/0x1a30 fs/ext4/extents.c:4597 >>> [<ffffffff8133610a>] ext4_map_blocks+0x1da/0x7b0 fs/ext4/inode.c:592 >>> [< inlined >] ext4_writepages+0x976/0x1480 >>> mpage_map_one_extent fs/ext4/inode.c:2109 >>> [< inlined >] ext4_writepages+0x976/0x1480 >>> mpage_map_and_submit_extent fs/ext4/inode.c:2165 >>> [<ffffffff8133b7b6>] ext4_writepages+0x976/0x1480 fs/ext4/inode.c:2508 >>> [<ffffffff811dbd23>] do_writepages+0x53/0x80 mm/page-writeback.c:2332 >>> [<ffffffff812a76bf>] __writeback_single_inode+0x7f/0x530 >>> fs/fs-writeback.c:1259 (discriminator 3) >>> [<ffffffff812a7fd4>] writeback_sb_inodes+0x464/0x690 fs/fs-writeback.c:1516 >>> [<ffffffff812a82c1>] __writeback_inodes_wb+0xc1/0x100 fs/fs-writeback.c:1562 >>> [<ffffffff812a86ae>] wb_writeback+0x3ae/0x450 fs/fs-writeback.c:1666 >>> [< inlined >] wb_workfn+0x203/0x780 wb_do_writeback >>> fs/fs-writeback.c:1801 >>> [<ffffffff812a91c3>] wb_workfn+0x203/0x780 fs/fs-writeback.c:1852 >>> [<ffffffff810b06ce>] process_one_work+0x28e/0x710 kernel/workqueue.c:2036 >>> [<ffffffff810b1299>] worker_thread+0xb9/0x750 kernel/workqueue.c:2170 >>> [<ffffffff810b9c61>] kthread+0x161/0x180 kernel/kthread.c:209 >>> [<ffffffff81eb0a1f>] ret_from_fork+0x3f/0x70 arch/x86/entry/entry_64.S:526 >>> >>> Previous read of size 8 by thread T512 (K7200): >>> [< inlined >] ext4_mark_iloc_dirty+0x454/0xe20 >>> ext4_inode_blocks_set fs/ext4/inode.c:4272 >>> [< inlined >] ext4_mark_iloc_dirty+0x454/0xe20 >>> ext4_do_update_inode fs/ext4/inode.c:4430 >>> [<ffffffff8133a014>] ext4_mark_iloc_dirty+0x454/0xe20 fs/ext4/inode.c:4937 >>> [<ffffffff8133ab8b>] ext4_mark_inode_dirty+0xdb/0x390 fs/ext4/inode.c:5053 >>> [<ffffffff8133f929>] ext4_dirty_inode+0x59/0x80 fs/ext4/inode.c:5085 >>> [<ffffffff812a7319>] __mark_inode_dirty+0x2c9/0x5f0 fs/fs-writeback.c:2015 >>> [<ffffffff8128a58e>] generic_update_time+0xbe/0x150 fs/inode.c:1566 >>> [< inlined >] file_update_time+0x112/0x1b0 update_time fs/inode.c:1582 >>> [<ffffffff812890f2>] file_update_time+0x112/0x1b0 fs/inode.c:1785 >>> [<ffffffff811cb175>] __generic_file_write_iter+0x105/0x2e0 mm/filemap.c:2570 >>> [<ffffffff8132c3a4>] ext4_file_write_iter+0x254/0x740 fs/ext4/file.c:170 >>> [< inlined >] __vfs_write+0x19c/0x1e0 new_sync_write fs/read_write.c:478 >>> [<ffffffff8125d48c>] __vfs_write+0x19c/0x1e0 fs/read_write.c:491 >>> [<ffffffff8125dde6>] vfs_write+0xf6/0x2a0 fs/read_write.c:538 >>> [< inlined >] SyS_write+0x6b/0xd0 SYSC_write fs/read_write.c:585 >>> [<ffffffff8125f37b>] SyS_write+0x6b/0xd0 fs/read_write.c:577 >>> [<ffffffff81eb062e>] entry_SYSCALL_64_fastpath+0x12/0x71 >>> arch/x86/entry/entry_64.S:186 >>> ================================================================== >>> >>> The 'inode->i_blocks' field is updated in one thread, while being read >>> and used in another. >>> >>> This can probably be fixed with a few READ_ONCE/WRITE_ONCE or by >>> taking inode->i_lock in ext4_inode_blocks_set. >> >> Yeah, the right fix would be to use inode->i_lock as quota code does, >> possibly with a wrapper function so that it can be avoided for 64-bit archs >> (see how i_size_read() / i_size_write() gets handled). However if you're >> going to fix this (and I'd note that this race is mostly theoretical since >> it would require 32-bit architecture and a file using more than 2TB of >> space) it's not just about ext4_inode_blocks_set() but about auditing all >> the other places working with i_blocks which is kind of a pain given the >> theoretical nature of the race... >> >> Honza >> -- >> Jan Kara <jack@suse.com> >> SUSE Labs, CR ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Data race in __inode_add_bytes 2015-09-25 9:51 ` Dmitry Vyukov @ 2015-09-25 11:52 ` Jan Kara 0 siblings, 0 replies; 4+ messages in thread From: Jan Kara @ 2015-09-25 11:52 UTC (permalink / raw) To: Dmitry Vyukov Cc: Jan Kara, Andrey Konovalov, Alexander Viro, linux-fsdevel, Theodore Ts'o, Andreas Dilger, linux-ext4, Aneesh Kumar K.V, Alexander Potapenko, LKML, ktsan On Fri 25-09-15 11:51:53, Dmitry Vyukov wrote: > On Fri, Sep 25, 2015 at 11:36 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > > Looking at __inode_add_bytes/ext4_inode_blocks_set I see much more > > ways to screw things up. > > For example, __inode_add_bytes: > > > > void __inode_add_bytes(struct inode *inode, loff_t bytes) > > { > > inode->i_blocks += bytes >> 9; > > bytes &= 511; > > inode->i_bytes += bytes; > > if (inode->i_bytes >= 512) { > > inode->i_blocks++; > > inode->i_bytes -= 512; > > } > > } > > > > can be compiled effectively as: > > > > void __inode_add_bytes(struct inode *inode, loff_t bytes) > > { > > inode->i_blocks += bytes >> 9 + 1; > > bytes = inode->i_bytes + (bytes & 511); > > if (bytes < 512) > > inode->i_blocks--; > > inode->i_bytes = bytes & 511; > > } > > > > Which will produce invalid results on any bitness with any file size. Yeah, strictly speaking compiler could screw us up like you describe but I'm yet to see such compiler especially since in ext4 case "bytes & 511" is always 0 similarly as inode->i_bytes. Anyway, I agree reading i_blocks under inode->i_lock would be desirable but frankly it's pretty low on my todo list... Honza > > On Fri, Sep 25, 2015 at 11:00 AM, Jan Kara <jack@suse.cz> wrote: > >> On Mon 31-08-15 21:33:46, Andrey Konovalov wrote: > >>> Hi! > >>> > >>> We are working on a dynamic data race detector for the Linux kernel, > >>> KernelThreadSanitizer (ktsan): > >>> https://github.com/google/ktsan/wiki > >>> > >>> We got a report while running ktsan on 4.2: > >>> > >>> ================================================================== > >>> ThreadSanitizer: data-race in __inode_add_bytes > >>> > >>> Write of size 8 by thread T210 (K740): > >>> [<ffffffff81266435>] __inode_add_bytes+0x55/0xd0 fs/stat.c:451 > >>> [<ffffffff812f90c0>] inode_claim_rsv_space+0x60/0xa0 fs/quota/dquot.c:1557 > >>> [<ffffffff812f9f7b>] dquot_claim_space_nodirty+0x3b/0x280 fs/quota/dquot.c:1721 > >>> [< inlined >] ext4_da_update_reserve_space+0x13b/0x2c0 > >>> dquot_claim_block include/linux/quotaops.h:345 > >>> [<ffffffff81335dab>] ext4_da_update_reserve_space+0x13b/0x2c0 > >>> fs/ext4/inode.c:350 > >>> [<ffffffff81384cf0>] ext4_ext_map_blocks+0x1570/0x1a30 fs/ext4/extents.c:4597 > >>> [<ffffffff8133610a>] ext4_map_blocks+0x1da/0x7b0 fs/ext4/inode.c:592 > >>> [< inlined >] ext4_writepages+0x976/0x1480 > >>> mpage_map_one_extent fs/ext4/inode.c:2109 > >>> [< inlined >] ext4_writepages+0x976/0x1480 > >>> mpage_map_and_submit_extent fs/ext4/inode.c:2165 > >>> [<ffffffff8133b7b6>] ext4_writepages+0x976/0x1480 fs/ext4/inode.c:2508 > >>> [<ffffffff811dbd23>] do_writepages+0x53/0x80 mm/page-writeback.c:2332 > >>> [<ffffffff812a76bf>] __writeback_single_inode+0x7f/0x530 > >>> fs/fs-writeback.c:1259 (discriminator 3) > >>> [<ffffffff812a7fd4>] writeback_sb_inodes+0x464/0x690 fs/fs-writeback.c:1516 > >>> [<ffffffff812a82c1>] __writeback_inodes_wb+0xc1/0x100 fs/fs-writeback.c:1562 > >>> [<ffffffff812a86ae>] wb_writeback+0x3ae/0x450 fs/fs-writeback.c:1666 > >>> [< inlined >] wb_workfn+0x203/0x780 wb_do_writeback > >>> fs/fs-writeback.c:1801 > >>> [<ffffffff812a91c3>] wb_workfn+0x203/0x780 fs/fs-writeback.c:1852 > >>> [<ffffffff810b06ce>] process_one_work+0x28e/0x710 kernel/workqueue.c:2036 > >>> [<ffffffff810b1299>] worker_thread+0xb9/0x750 kernel/workqueue.c:2170 > >>> [<ffffffff810b9c61>] kthread+0x161/0x180 kernel/kthread.c:209 > >>> [<ffffffff81eb0a1f>] ret_from_fork+0x3f/0x70 arch/x86/entry/entry_64.S:526 > >>> > >>> Previous read of size 8 by thread T512 (K7200): > >>> [< inlined >] ext4_mark_iloc_dirty+0x454/0xe20 > >>> ext4_inode_blocks_set fs/ext4/inode.c:4272 > >>> [< inlined >] ext4_mark_iloc_dirty+0x454/0xe20 > >>> ext4_do_update_inode fs/ext4/inode.c:4430 > >>> [<ffffffff8133a014>] ext4_mark_iloc_dirty+0x454/0xe20 fs/ext4/inode.c:4937 > >>> [<ffffffff8133ab8b>] ext4_mark_inode_dirty+0xdb/0x390 fs/ext4/inode.c:5053 > >>> [<ffffffff8133f929>] ext4_dirty_inode+0x59/0x80 fs/ext4/inode.c:5085 > >>> [<ffffffff812a7319>] __mark_inode_dirty+0x2c9/0x5f0 fs/fs-writeback.c:2015 > >>> [<ffffffff8128a58e>] generic_update_time+0xbe/0x150 fs/inode.c:1566 > >>> [< inlined >] file_update_time+0x112/0x1b0 update_time fs/inode.c:1582 > >>> [<ffffffff812890f2>] file_update_time+0x112/0x1b0 fs/inode.c:1785 > >>> [<ffffffff811cb175>] __generic_file_write_iter+0x105/0x2e0 mm/filemap.c:2570 > >>> [<ffffffff8132c3a4>] ext4_file_write_iter+0x254/0x740 fs/ext4/file.c:170 > >>> [< inlined >] __vfs_write+0x19c/0x1e0 new_sync_write fs/read_write.c:478 > >>> [<ffffffff8125d48c>] __vfs_write+0x19c/0x1e0 fs/read_write.c:491 > >>> [<ffffffff8125dde6>] vfs_write+0xf6/0x2a0 fs/read_write.c:538 > >>> [< inlined >] SyS_write+0x6b/0xd0 SYSC_write fs/read_write.c:585 > >>> [<ffffffff8125f37b>] SyS_write+0x6b/0xd0 fs/read_write.c:577 > >>> [<ffffffff81eb062e>] entry_SYSCALL_64_fastpath+0x12/0x71 > >>> arch/x86/entry/entry_64.S:186 > >>> ================================================================== > >>> > >>> The 'inode->i_blocks' field is updated in one thread, while being read > >>> and used in another. > >>> > >>> This can probably be fixed with a few READ_ONCE/WRITE_ONCE or by > >>> taking inode->i_lock in ext4_inode_blocks_set. > >> > >> Yeah, the right fix would be to use inode->i_lock as quota code does, > >> possibly with a wrapper function so that it can be avoided for 64-bit archs > >> (see how i_size_read() / i_size_write() gets handled). However if you're > >> going to fix this (and I'd note that this race is mostly theoretical since > >> it would require 32-bit architecture and a file using more than 2TB of > >> space) it's not just about ext4_inode_blocks_set() but about auditing all > >> the other places working with i_blocks which is kind of a pain given the > >> theoretical nature of the race... > >> > >> Honza > >> -- > >> Jan Kara <jack@suse.com> > >> SUSE Labs, CR -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-09-25 11:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CAAeHK+womwjgo4UgErWzDC9XD7BwUB1TkaFLo0eC7jF0ftV6gg@mail.gmail.com>
2015-08-31 19:35 ` Fwd: Data race in __inode_add_bytes Andrey Konovalov
2015-09-01 9:21 ` Dmitry Vyukov
[not found] ` <20150925090016.GB2806@quack.suse.cz>
[not found] ` <CACT4Y+bWXRnNwmv+p2wKQY+c9e0VOsCqSkUiLa_1s54gCV7HJg@mail.gmail.com>
2015-09-25 9:51 ` Dmitry Vyukov
2015-09-25 11:52 ` Jan Kara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®