* Possible circular dependency between i_data_sem and folio lock in ext4 filesystem
@ 2024-07-11 12:07 Hyeonggon Yoo
2024-07-11 15:38 ` Theodore Ts'o
0 siblings, 1 reply; 6+ messages in thread
From: Hyeonggon Yoo @ 2024-07-11 12:07 UTC (permalink / raw)
To: Linux Memory Management List, linux-fsdevel, linux-ext4
Cc: max.byungchul.park, byungchul, Gwan-gyeong Mun
Hi folks,
Byungchul, Gwan-gyeong and I are investigating possible circular
dependency reported by a dependency tracker named DEPT [1], which is
able to report possible circular dependencies involving folio locks
and other forms of dependencies that are not locks (i.e., wait for
completion).
Below are two similar reports from DEPT where one context takes
i_data_sem and then folio lock in ext4_map_blocks(), while the other
context takes folio lock and then i_data_sem during processing of
pwrite64() system calls. We're reaching out due to a lack of
understanding of ext4 and file system internals.
The points in question are:
- Can the two contexts actually create a dependency between each other
in ext4? In other words, do their uses of folio lock make them belong
to the same lock classes?
- Are there any locking rules in ext4 that ensure these two contexts
will never be considered as the same lock class?
Best,
Hyeonggon
==== Report 1: Possible circular dependency between fsync() and pwrite64() ====
[ 402.888557] ===================================================
[ 402.888967] DEPT: Circular dependency has been detected.
[ 402.888967] 6.9.0-rc7 #145 Not tainted
[ 402.888967] ---------------------------------------------------
[ 402.888967] summary
[ 402.888967] ---------------------------------------------------
[ 402.888967] *** DEADLOCK ***
[ 402.888967]
[ 402.888967] context A
[ 402.888967] [S] lock(&ei->i_data_sem:0)
[ 402.888967] [W] dept_page_wait_on_bit(PG_locked_map:0)
[ 402.888967] [E] unlock(&ei->i_data_sem:0)
[ 402.888967]
[ 402.888967] context B
[ 402.888967] [S] (event requestor)(PG_locked_map:0)
[ 402.888967] [W] lock(&ei->i_data_sem:0)
[ 402.888967] [E] dept_page_clear_bit(PG_locked_map:0)
[ 402.888967]
[ 402.888967] [S]: start of the event context
[ 402.888967] [W]: the wait blocked
[ 402.888967] [E]: the event not reachable
[ 402.888967] ---------------------------------------------------
[ 402.888967] context A's detail
[ 402.888967] ---------------------------------------------------
[ 402.888967] context A
[ 402.888967] [S] lock(&ei->i_data_sem:0)
[ 402.888967] [W] dept_page_wait_on_bit(PG_locked_map:0)
[ 402.888967] [E] unlock(&ei->i_data_sem:0)
[ 402.888967]
[ 402.888967] [S] lock(&ei->i_data_sem:0):
[ 402.888967] ext4_map_blocks (fs/ext4/ext4.h:1936 fs/ext4/inode.c:622)
[ 402.888967] stacktrace:
[ 402.888967] ext4_map_blocks (fs/ext4/ext4.h:1936 fs/ext4/inode.c:622)
[ 402.888967] ext4_do_writepages (fs/ext4/inode.c:2164
fs/ext4/inode.c:2216 fs/ext4/inode.c:2679)
[ 402.888967] ext4_writepages (fs/ext4/inode.c:2768)
[ 402.888967] do_writepages (mm/page-writeback.c:2619)
[ 402.888967] __filemap_fdatawrite_range (mm/filemap.c:431)
[ 402.888967] file_write_and_wait_range (mm/filemap.c:789)
[ 402.888967] ext4_sync_file (fs/ext4/fsync.c:158)
[ 402.888967] __x64_sys_fsync (./include/linux/file.h:47
fs/sync.c:213 fs/sync.c:220 fs/sync.c:218 fs/sync.c:218)
[ 402.888967] do_syscall_64 (arch/x86/entry/common.c:53
arch/x86/entry/common.c:85)
[ 402.888967] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[ 402.888967]
[ 402.888967] [W] dept_page_wait_on_bit(PG_locked_map:0):
[ 402.888967] __filemap_get_folio (./include/linux/pagemap.h:1054
./include/linux/pagemap.h:1049 mm/filemap.c:1900)
[ 402.888967] stacktrace:
[ 402.888967] folio_wait_bit_common (./include/linux/page-flags.h:873
mm/filemap.c:1224)
[ 402.888967] __filemap_get_folio (./include/linux/pagemap.h:1054
./include/linux/pagemap.h:1049 mm/filemap.c:1900)
[ 402.888967] pagecache_get_page (mm/folio-compat.c:94)
[ 402.888967] ext4_mb_load_buddy_gfp (./include/linux/pagemap.h:757
fs/ext4/mballoc.c:1634)
[ 402.888967] ext4_mb_regular_allocator (fs/ext4/mballoc.c:2898)
[ 402.888967] ext4_mb_new_blocks (fs/ext4/mballoc.c:6205)
[ 402.888967] ext4_ext_map_blocks (fs/ext4/extents.c:4317)
[ 402.888967] ext4_map_blocks (fs/ext4/inode.c:623)
[ 402.888967] ext4_do_writepages (fs/ext4/inode.c:2164
fs/ext4/inode.c:2216 fs/ext4/inode.c:2679)
[ 402.888967] ext4_writepages (fs/ext4/inode.c:2768)
[ 402.888967] do_writepages (mm/page-writeback.c:2619)
[ 402.888967] __filemap_fdatawrite_range (mm/filemap.c:431)
[ 402.888967] file_write_and_wait_range (mm/filemap.c:789)
[ 402.888967] ext4_sync_file (fs/ext4/fsync.c:158)
[ 402.888967] __x64_sys_fsync (./include/linux/file.h:47
fs/sync.c:213 fs/sync.c:220 fs/sync.c:218 fs/sync.c:218)
[ 402.888967] do_syscall_64 (arch/x86/entry/common.c:53
arch/x86/entry/common.c:85)
[ 402.888967]
[ 402.888967] [E] unlock(&ei->i_data_sem:0):
[ 402.888967] (N/A)
[ 402.888967] ---------------------------------------------------
[ 402.888967] context B's detail
[ 402.888967] ---------------------------------------------------
[ 402.888967] context B
[ 402.888967] [S] (event requestor)(PG_locked_map:0)
[ 402.888967] [W] lock(&ei->i_data_sem:0)
[ 402.888967] [E] dept_page_clear_bit(PG_locked_map:0)
[ 402.888967]
[ 402.888967] [S] (event requestor)(PG_locked_map:0):
[ 402.888967] stacktrace:
[ 402.888967] filemap_add_folio (mm/filemap.c:948)
[ 402.888967] __filemap_get_folio (mm/filemap.c:1960)
[ 402.888967] ext4_da_write_begin (fs/ext4/inode.c:2885)
[ 402.888967] generic_perform_write (mm/filemap.c:4000)
[ 402.888967] ext4_buffered_write_iter (./include/linux/fs.h:800
fs/ext4/file.c:302)
[ 402.888967] ext4_file_write_iter (fs/ext4/file.c:698)
[ 402.888967] vfs_write (./include/linux/fs.h:2110
fs/read_write.c:497 fs/read_write.c:590)
[ 402.888967] __x64_sys_pwrite64 (fs/read_write.c:705
fs/read_write.c:715 fs/read_write.c:712 fs/read_write.c:712)
[ 402.888967] do_syscall_64 (arch/x86/entry/common.c:53
arch/x86/entry/common.c:85)
[ 402.888967] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[ 402.888967]
[ 402.888967] [W] lock(&ei->i_data_sem:0):
[ 402.888967] ext4_da_get_block_prep (fs/ext4/ext4.h:1936
fs/ext4/ext4.h:3600 fs/ext4/inode.c:1745 fs/ext4/inode.c:1817)
[ 402.888967] stacktrace:
[ 402.888967] ext4_da_get_block_prep (fs/ext4/ext4.h:1936
fs/ext4/ext4.h:3600 fs/ext4/inode.c:1745 fs/ext4/inode.c:1817)
[ 402.888967] __block_write_begin_int (fs/buffer.c:2108)
[ 402.888967] ext4_da_write_begin (fs/ext4/inode.c:2896)
[ 402.888967] generic_perform_write (mm/filemap.c:4000)
[ 402.888967] ext4_buffered_write_iter (./include/linux/fs.h:800
fs/ext4/file.c:302)
[ 402.888967] ext4_file_write_iter (fs/ext4/file.c:698)
[ 402.888967] vfs_write (./include/linux/fs.h:2110
fs/read_write.c:497 fs/read_write.c:590)
[ 402.888967] __x64_sys_pwrite64 (fs/read_write.c:705
fs/read_write.c:715 fs/read_write.c:712 fs/read_write.c:712)
[ 402.888967] do_syscall_64 (arch/x86/entry/common.c:53
arch/x86/entry/common.c:85)
[ 402.888967] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[ 402.888967]
[ 402.888967] [E] dept_page_clear_bit(PG_locked_map:0):
[ 402.888967] ext4_da_write_end (./include/linux/instrumented.h:68
./include/linux/atomic/atomic-instrumented.h:32
./include/linux/page_ref.h:67 ./include/linux/mm.h:1134
./include/linux/mm.h:1140 ./include/linux/mm.h:1507
fs/ext4/inode.c:2986 fs/ext4/inode.c:3028)
[ 402.888967] stacktrace:
[ 402.888967] folio_unlock (./include/linux/page-flags.h:858
(discriminator 2) mm/filemap.c:1510 (discriminator 2))
[ 402.888967] ext4_da_write_end (./include/linux/instrumented.h:68
./include/linux/atomic/atomic-instrumented.h:32
./include/linux/page_ref.h:67 ./include/linux/mm.h:1134
./include/linux/mm.h:1140 ./include/linux/mm.h:1507
fs/ext4/inode.c:2986 fs/ext4/inode.c:3028)
[ 402.888967] generic_perform_write (mm/filemap.c:4011)
[ 402.888967] ext4_buffered_write_iter (./include/linux/fs.h:800
fs/ext4/file.c:302)
[ 402.888967] ext4_file_write_iter (fs/ext4/file.c:698)
[ 402.888967] vfs_write (./include/linux/fs.h:2110
fs/read_write.c:497 fs/read_write.c:590)
[ 402.888967] __x64_sys_pwrite64 (fs/read_write.c:705
fs/read_write.c:715 fs/read_write.c:712 fs/read_write.c:712)
[ 402.888967] do_syscall_64 (arch/x86/entry/common.c:53
arch/x86/entry/common.c:85)
[ 402.888967] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[ 402.888967] ---------------------------------------------------
[ 402.888967] information that might be helpful
[ 402.888967] ---------------------------------------------------
[ 402.888967] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.12.0-1 04/01/2014
[ 402.888967] Call Trace:
[ 402.888967] <TASK>
[ 402.888967] dump_stack_lvl (lib/dump_stack.c:118)
[ 402.888967] print_circle (./include/linux/instrumented.h:96
./include/linux/atomic/atomic-instrumented.h:592
kernel/dependency/dept.c:149 kernel/dependency/dept.c:886)
[ 402.888967] ? hlock_class (./arch/x86/include/asm/bitops.h:227
./arch/x86/include/asm/bitops.h:239
./include/asm-generic/bitops/instrumented-non-atomic.h:142
kernel/locking/lockdep.c:228)
[ 402.888967] ? extend_queue (./include/linux/list.h:150
./include/linux/list.h:183 kernel/dependency/dept.c:898
kernel/dependency/dept.c:927)
[ 402.888967] cb_check_dl (kernel/dependency/dept.c:1244)
[ 402.888967] bfs (kernel/dependency/dept.c:984)
[ 402.888967] ? __pfx_cb_check_dl (kernel/dependency/dept.c:1220)
[ 402.888967] ? hlock_class (./arch/x86/include/asm/bitops.h:227
./arch/x86/include/asm/bitops.h:239
./include/asm-generic/bitops/instrumented-non-atomic.h:142
kernel/locking/lockdep.c:228)
[ 402.888967] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 402.888967] ? mark_lock (kernel/locking/lockdep.c:4666)
[ 402.888967] ? __pfx_bfs (kernel/dependency/dept.c:954)
[ 402.888967] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 402.888967] ? __add_dep (./include/linux/list.h:150
./include/linux/list.h:169 kernel/dependency/dept.c:1214)
[ 402.888967] add_dep (kernel/dependency/dept.c:1579)
[ 402.888967] ? hlock_class (./arch/x86/include/asm/bitops.h:227
./arch/x86/include/asm/bitops.h:239
./include/asm-generic/bitops/instrumented-non-atomic.h:142
kernel/locking/lockdep.c:228)
[ 402.888967] ? __pfx_add_dep (kernel/dependency/dept.c:1560)
[ 402.888967] ? __pfx_from_pool (kernel/dependency/dept.c:351)
[ 402.888967] ? __pfx_mark_lock (kernel/locking/lockdep.c:4649)
[ 402.888967] ? hlock_class (./arch/x86/include/asm/bitops.h:227
./arch/x86/include/asm/bitops.h:239
./include/asm-generic/bitops/instrumented-non-atomic.h:142
kernel/locking/lockdep.c:228)
[ 402.888967] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 402.888967] __dept_wait (kernel/dependency/dept.c:1641
kernel/dependency/dept.c:2381)
[ 402.888967] ? __filemap_get_folio (./include/linux/pagemap.h:1054
./include/linux/pagemap.h:1049 mm/filemap.c:1900)
[ 402.888967] ? __pfx___lock_acquire (kernel/locking/lockdep.c:5005)
[ 402.888967] ? __lock_acquire (kernel/locking/lockdep.c:5146
(discriminator 9))
[ 402.888967] ? __pfx___dept_wait (kernel/dependency/dept.c:2359)
[ 402.888967] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 402.888967] ? lock_acquire (kernel/locking/lockdep.c:467
kernel/locking/lockdep.c:5776 kernel/locking/lockdep.c:5739)
[ 402.888967] ? __filemap_get_folio (./include/linux/pagemap.h:1054
./include/linux/pagemap.h:1049 mm/filemap.c:1900)
[ 403.025017] dept_wait (kernel/dependency/dept.c:2446)
[ 403.025017] folio_wait_bit_common (./include/linux/page-flags.h:873
mm/filemap.c:1224)
[ 403.025017] ? __pfx_folio_wait_bit_common (mm/filemap.c:1212)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? filemap_get_entry (mm/filemap.c:1835)
[ 403.025017] ? __pfx_filemap_get_entry (mm/filemap.c:1835)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? lock_is_held_type (kernel/locking/lockdep.c:5507
kernel/locking/lockdep.c:5845)
[ 403.025017] __filemap_get_folio (./include/linux/pagemap.h:1054
./include/linux/pagemap.h:1049 mm/filemap.c:1900)
[ 403.025017] pagecache_get_page (mm/folio-compat.c:94)
[ 403.025017] ext4_mb_load_buddy_gfp (./include/linux/pagemap.h:757
fs/ext4/mballoc.c:1634)
[ 403.025017] ext4_mb_regular_allocator (fs/ext4/mballoc.c:2898)
[ 403.025017] ? __pfx_ext4_mb_regular_allocator (fs/ext4/mballoc.c:2786)
[ 403.025017] ? ext4_mb_normalize_request.constprop.0 (fs/ext4/mballoc.c:4540)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? __kasan_slab_alloc (mm/kasan/common.c:341)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? kmem_cache_alloc (mm/slub.c:3858)
[ 403.025017] ext4_mb_new_blocks (fs/ext4/mballoc.c:6205)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? kasan_save_track (./arch/x86/include/asm/current.h:49
mm/kasan/common.c:60 mm/kasan/common.c:69)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? __kasan_kmalloc (mm/kasan/common.c:391)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? __kmalloc (mm/slub.c:3980)
[ 403.025017] ? __pfx_ext4_mb_new_blocks (fs/ext4/mballoc.c:6126)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? ext4_find_extent (fs/ext4/extents.c:815 fs/ext4/extents.c:953)
[ 403.025017] ext4_ext_map_blocks (fs/ext4/extents.c:4317)
[ 403.025017] ? __is_insn_slot_addr (kernel/kprobes.c:312)
[ 403.025017] ? __pfx___lock_acquire (kernel/locking/lockdep.c:5005)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? __kernel_text_address (kernel/extable.c:79)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? unwind_get_return_address
(arch/x86/kernel/unwind_orc.c:369 arch/x86/kernel/unwind_orc.c:364)
[ 403.025017] ? __pfx_stack_trace_consume_entry (kernel/stacktrace.c:83)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? arch_stack_walk (arch/x86/kernel/stacktrace.c:26)
[ 403.025017] ? __pfx_ext4_ext_map_blocks (fs/ext4/extents.c:4128)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? ext4_map_blocks (fs/ext4/ext4.h:1936 fs/ext4/inode.c:622)
[ 403.025017] ? __pfx_lock_acquire (kernel/locking/lockdep.c:5742)
[ 403.025017] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.025017] ? stack_trace_save (kernel/stacktrace.c:123)
[ 403.045075] ? __pfx_stack_trace_save (kernel/stacktrace.c:114)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? dept_ecxt_enter (kernel/dependency/dept.c:2768
(discriminator 1))
[ 403.045075] ? __pfx_down_write (kernel/locking/rwsem.c:1577)
[ 403.045075] ? ext4_es_lookup_extent
(./include/trace/events/ext4.h:2313 fs/ext4/extents_status.c:1047)
[ 403.045075] ext4_map_blocks (fs/ext4/inode.c:623)
[ 403.045075] ? __pfx_ext4_map_blocks (fs/ext4/inode.c:481)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? __kasan_slab_alloc (mm/kasan/common.c:341)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? ext4_alloc_io_end_vec (fs/ext4/page-io.c:61)
[ 403.045075] ext4_do_writepages (fs/ext4/inode.c:2164
fs/ext4/inode.c:2216 fs/ext4/inode.c:2679)
[ 403.045075] ? __pfx_ext4_do_writepages (fs/ext4/inode.c:2512)
[ 403.045075] ? stack_trace_save (kernel/stacktrace.c:123)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? dept_ecxt_enter (kernel/dependency/dept.c:2768
(discriminator 1))
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? lock_is_held_type (kernel/locking/lockdep.c:5507
kernel/locking/lockdep.c:5845)
[ 403.045075] ext4_writepages (fs/ext4/inode.c:2768)
[ 403.045075] ? __pfx_ext4_writepages (fs/ext4/inode.c:2754)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? ext4_buffered_write_iter (fs/ext4/file.c:303)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? lock_release (kernel/locking/lockdep.c:5442
kernel/locking/lockdep.c:5794)
[ 403.045075] do_writepages (mm/page-writeback.c:2619)
[ 403.045075] ? dept_ecxt_exit (kernel/dependency/dept.c:2948)
[ 403.045075] ? __pfx_do_writepages (mm/page-writeback.c:2602)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? up_write (./arch/x86/include/asm/atomic64_64.h:91
./include/linux/atomic/atomic-arch-fallback.h:2852
./include/linux/atomic/atomic-long.h:268
./include/linux/atomic/atomic-instrumented.h:3391
kernel/locking/rwsem.c:1374 kernel/locking/rwsem.c:1632)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? llist_add_batch (lib/llist.c:33 (discriminator 14))
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? llist_add_batch (lib/llist.c:33 (discriminator 14))
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? hlock_class (./arch/x86/include/asm/bitops.h:227
./arch/x86/include/asm/bitops.h:239
./include/asm-generic/bitops/instrumented-non-atomic.h:142
kernel/locking/lockdep.c:228)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] ? __lock_acquire (kernel/locking/lockdep.c:5146
(discriminator 9))
[ 403.045075] __filemap_fdatawrite_range (mm/filemap.c:431)
[ 403.045075] ? __pfx___filemap_fdatawrite_range (mm/filemap.c:423)
[ 403.045075] file_write_and_wait_range (mm/filemap.c:789)
[ 403.045075] ext4_sync_file (fs/ext4/fsync.c:158)
[ 403.045075] ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
[ 403.045075] __x64_sys_fsync (./include/linux/file.h:47
fs/sync.c:213 fs/sync.c:220 fs/sync.c:218 fs/sync.c:218)
[ 403.045075] do_syscall_64 (arch/x86/entry/common.c:53
arch/x86/entry/common.c:85)
[ 403.045075] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[ 403.045075] RIP: 0033:0x7f93ccae65eb
[ 403.045075] RSP: 002b:00007f93c1dfde50 EFLAGS: 00000293 ORIG_RAX:
000000000000004a
[ 403.045075] RAX: ffffffffffffffda RBX: 00000000003e0000 RCX: 00007f93ccae65eb
[ 403.045075] RDX: 000055f93c6e639d RSI: 00007f93c1dfde70 RDI: 0000000000000014
[ 403.045075] RBP: 00007f93c1dffebc R08: 0000000000000000 R09: 00007f93c1dfde70
[ 403.045075] R10: 0000000000000180 R11: 0000000000000293 R12: 0000000000000014
[ 403.045075] R13: 00007f93c1dfde70 R14: 000055f93c702004 R15: 0000000000802000
[ 403.045075] </TASK>
==== Report 2: Possible circular dependency between mkdir() and pwrite64()
[ 19.908810] DEPT: Circular dependency has been detected.
[ 19.908810] 6.9.0-rc7 #52 Not tainted
[ 19.908810] ---------------------------------------------------
[ 19.908810] summary
[ 19.908810] ---------------------------------------------------
[ 19.908810] *** DEADLOCK ***
[ 19.908810]
[ 19.908810] context A
[ 19.908810] [S] lock(&ei->i_data_sem:0)
[ 19.908810] [W] dept_page_wait_on_bit(PG_locked_map:0)
[ 19.908810] [E] unlock(&ei->i_data_sem:0)
[ 19.908810]
[ 19.908810] context B
[ 19.908810] [S] (event requestor)(PG_locked_map:0)
[ 19.908810] [W] lock(&ei->i_data_sem:0)
[ 19.908810] [E] dept_page_clear_bit(PG_locked_map:0)
[ 19.908810]
[ 19.908810] [S]: start of the event context
[ 19.908810] [W]: the wait blocked
[ 19.908810] [E]: the event not reachable
[ 19.908810] ---------------------------------------------------
[ 19.908810] context A's detail
[ 19.908810] ---------------------------------------------------
[ 19.908810] context A
[ 19.908810] [S] lock(&ei->i_data_sem:0)
[ 19.908810] [W] dept_page_wait_on_bit(PG_locked_map:0)
[ 19.908810] [E] unlock(&ei->i_data_sem:0)
[ 19.908810]
[ 19.908810] [S] lock(&ei->i_data_sem:0):
[ 19.908810] [<ffffffffa83e460d>] ext4_map_blocks+0x20d/0x5f0
[ 19.908810] stacktrace:
[ 19.908810] ext4_map_blocks+0x20d/0x5f0
[ 19.908810] ext4_getblk+0x7a/0x2e0
[ 19.908810] ext4_bread+0xb/0x70
[ 19.908810] ext4_append+0x88/0x190
[ 19.908810] ext4_init_new_dir+0xd6/0x180
[ 19.908810] ext4_mkdir+0x19d/0x340
[ 19.908810] vfs_mkdir+0x151/0x230
[ 19.908810] do_mkdirat+0x85/0x130
[ 19.908810] __x64_sys_mkdir+0x46/0x70
[ 19.908810] do_syscall_64+0xc8/0x1d0
[ 19.908810] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 19.908810]
[ 19.908810] [W] dept_page_wait_on_bit(PG_locked_map:0):
[ 19.908810] [<ffffffffa823de44>] __filemap_get_folio+0x1f4/0x240
[ 19.908810] stacktrace:
[ 19.908810] folio_wait_bit_common+0x289/0x360
[ 19.908810] __filemap_get_folio+0x1f4/0x240
[ 19.908810] pagecache_get_page+0xd/0x40
[ 19.908810] ext4_mb_init_group+0x73/0x320
[ 19.908810] ext4_mb_prefetch_fini+0x7d/0xa0
[ 19.908810] ext4_mb_regular_allocator+0x4c0/0xde0
[ 19.908810] ext4_mb_new_blocks+0xa04/0x10d0
[ 19.908810] ext4_ind_map_blocks+0x7e5/0xbb0
[ 19.908810] ext4_map_blocks+0x2da/0x5f0
[ 19.908810] ext4_getblk+0x7a/0x2e0
[ 19.908810] ext4_bread+0xb/0x70
[ 19.908810] ext4_append+0x88/0x190
[ 19.908810] ext4_init_new_dir+0xd6/0x180
[ 19.908810] ext4_mkdir+0x19d/0x340
[ 19.908810] vfs_mkdir+0x151/0x230
[ 19.908810] do_mkdirat+0x85/0x130
[ 19.908810]
[ 19.908810] [E] unlock(&ei->i_data_sem:0):
[ 19.908810] (N/A)
[ 19.908810] ---------------------------------------------------
[ 19.908810] context B's detail
[ 19.908810] ---------------------------------------------------
[ 19.908810] context B
[ 19.908810] [S] (event requestor)(PG_locked_map:0)
[ 19.908810] [W] lock(&ei->i_data_sem:0)
[ 19.908810] [E] dept_page_clear_bit(PG_locked_map:0)
[ 19.908810]
[ 19.908810] [S] (event requestor)(PG_locked_map:0):
[ 19.908810] stacktrace:
[ 19.908810] __filemap_get_folio+0xdf/0x240
[ 19.908810] ext4_da_write_begin+0xf1/0x2b0
[ 19.908810] generic_perform_write+0xca/0x210
[ 19.908810] ext4_buffered_write_iter+0x5d/0xe0
[ 19.908810] vfs_write+0x3d3/0x580
[ 19.908810] __x64_sys_pwrite64+0x92/0xc0
[ 19.908810] do_syscall_64+0xc8/0x1d0
[ 19.908810] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 19.908810]
[ 19.908810] [W] lock(&ei->i_data_sem:0):
[ 19.908810] [<ffffffffa83e31d9>] ext4_da_get_block_prep+0x1b9/0x400
[ 19.908810] stacktrace:
[ 19.908810] ext4_da_get_block_prep+0x1b9/0x400
[ 19.908810] __block_write_begin_int+0x155/0x570
[ 19.908810] ext4_da_write_begin+0x11d/0x2b0
[ 19.908810] generic_perform_write+0xca/0x210
[ 19.908810] ext4_buffered_write_iter+0x5d/0xe0
[ 19.908810] vfs_write+0x3d3/0x580
[ 19.908810] __x64_sys_pwrite64+0x92/0xc0
[ 19.908810] do_syscall_64+0xc8/0x1d0
[ 19.908810] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 19.908810]
[ 19.908810] [E] dept_page_clear_bit(PG_locked_map:0):
[ 19.908810] [<ffffffffa83eb9bd>] ext4_da_write_end+0xbd/0x3b0
[ 19.908810] stacktrace:
[ 19.908810] ext4_da_write_end+0xbd/0x3b0
[ 19.908810] generic_perform_write+0x11c/0x210
[ 19.908810] ext4_buffered_write_iter+0x5d/0xe0
[ 19.908810] vfs_write+0x3d3/0x580
[ 19.908810] __x64_sys_pwrite64+0x92/0xc0
[ 19.908810] do_syscall_64+0xc8/0x1d0
[ 19.908810] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 19.908810] ---------------------------------------------------
[ 19.908810] information that might be helpful
[ 19.908810] ---------------------------------------------------
[ 19.908810] CPU: 0 PID: 378 Comm: mkdir Not tainted 6.9.0-rc7 #52
[ 19.908810] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.12.0-1 04/01/2014
[ 19.908810] Call Trace:
[ 19.908810] <TASK>
[ 19.908810] dump_stack_lvl+0x68/0xa0
[ 19.908810] print_circle+0x690/0x6b0
[ 19.908810] ? __pfx_cb_check_dl+0x10/0x10
[ 19.908810] cb_check_dl+0x5c/0x70
[ 19.908810] bfs+0xca/0x1a0
[ 19.908810] add_dep+0xaa/0x170
[ 19.908810] __dept_wait+0x1ef/0x580
[ 19.908810] ? __filemap_get_folio+0x1f4/0x240
[ 19.908810] dept_wait+0x9d/0xb0
[ 19.908810] ? __filemap_get_folio+0x1f4/0x240
[ 19.908810] folio_wait_bit_common+0x289/0x360
[ 19.908810] ? srso_return_thunk+0x5/0x5f
[ 19.908810] ? filemap_get_entry+0x109/0x1e0
[ 19.908810] __filemap_get_folio+0x1f4/0x240
[ 19.908810] pagecache_get_page+0xd/0x40
[ 19.908810] ext4_mb_init_group+0x73/0x320
[ 19.908810] ext4_mb_prefetch_fini+0x7d/0xa0
[ 19.908810] ext4_mb_regular_allocator+0x4c0/0xde0
[ 19.908810] ? lock_release+0xbd/0x280
[ 19.908810] ? fs_reclaim_acquire+0x4e/0xf0
[ 19.908810] ext4_mb_new_blocks+0xa04/0x10d0
[ 19.908810] ? srso_return_thunk+0x5/0x5f
[ 19.908810] ? srso_return_thunk+0x5/0x5f
[ 19.908810] ext4_ind_map_blocks+0x7e5/0xbb0
[ 19.908810] ? srso_return_thunk+0x5/0x5f
[ 19.908810] ? lock_acquire+0xc0/0x2d0
[ 19.908810] ? ext4_map_blocks+0x20d/0x5f0
[ 19.908810] ? srso_return_thunk+0x5/0x5f
[ 19.908810] ext4_map_blocks+0x2da/0x5f0
[ 19.908810] ? srso_return_thunk+0x5/0x5f
[ 19.908810] ? lock_release+0xbd/0x280
[ 19.908810] ? srso_return_thunk+0x5/0x5f
[ 19.908810] ext4_getblk+0x7a/0x2e0
[ 19.908810] ext4_bread+0xb/0x70
[ 19.908810] ext4_append+0x88/0x190
[ 19.908810] ext4_init_new_dir+0xd6/0x180
[ 19.908810] ext4_mkdir+0x19d/0x340
[ 19.908810] vfs_mkdir+0x151/0x230
[ 19.908810] do_mkdirat+0x85/0x130
[ 19.908810] __x64_sys_mkdir+0x46/0x70
[ 19.908810] do_syscall_64+0xc8/0x1d0
[ 19.908810] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 19.908810] RIP: 0033:0x7f4cbd4b4047
[ 19.908810] Code: 1f 40 00 48 8b 05 49 2e 0e 00 64 c7 00 5f 00 00
00 b8 ff ff ff ff c3 66 2e 0f 1f 84 0000 00 00 00 66 90 b8 53 00 00 00
0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 19 2e 0e 00 f7 d8 64 89 01
48
[ 19.908810] RSP: 002b:00007fffa8322d78 EFLAGS: 00000246 ORIG_RAX:
0000000000000053
[ 19.908810] RAX: ffffffffffffffda RBX: 000055947ace3c30 RCX: 00007f4cbd4b4047
[ 19.908810] RDX: 0000000000000000 RSI: 00000000000001ff RDI: 00007fffa8323e2d
[ 19.908810] RBP: 00007fffa8323e20 R08: 0000000000000000 R09: 000055947ace3d60
[ 19.908810] R10: fffffffffffffd8c R11: 0000000000000246 R12: 00000000000001ff
[ 19.908810] R13: 00007fffa8322ef0 R14: 00007fffa8323e2d R15: 0000000000000000
[ 19.908810] </TASK>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible circular dependency between i_data_sem and folio lock in ext4 filesystem
2024-07-11 12:07 Possible circular dependency between i_data_sem and folio lock in ext4 filesystem Hyeonggon Yoo
@ 2024-07-11 15:38 ` Theodore Ts'o
2024-07-12 4:44 ` Byungchul Park
0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2024-07-11 15:38 UTC (permalink / raw)
To: Hyeonggon Yoo
Cc: Linux Memory Management List, linux-fsdevel, linux-ext4,
max.byungchul.park, byungchul, Gwan-gyeong Mun
On Thu, Jul 11, 2024 at 09:07:53PM +0900, Hyeonggon Yoo wrote:
> Hi folks,
>
> Byungchul, Gwan-gyeong and I are investigating possible circular
> dependency reported by a dependency tracker named DEPT [1], which is
> able to report possible circular dependencies involving folio locks
> and other forms of dependencies that are not locks (i.e., wait for
> completion).
>
> Below are two similar reports from DEPT where one context takes
> i_data_sem and then folio lock in ext4_map_blocks(), while the other
> context takes folio lock and then i_data_sem during processing of
> pwrite64() system calls. We're reaching out due to a lack of
> understanding of ext4 and file system internals.
>
> The points in question are:
>
> - Can the two contexts actually create a dependency between each other
> in ext4? In other words, do their uses of folio lock make them belong
> to the same lock classes?
No.
> - Are there any locking rules in ext4 that ensure these two contexts
> will never be considered as the same lock class?
It's inherent is the code path. In one of the stack traces, we are
using the page cache for the bitmap allocation block (in other words, a metadata
block). In the other stack trace, the page cache belongs to a regular
file (in other words, a data block).
So this is a false positive with DEPT, which has always been one of
the reasons why I've been dubious about the value of DEPT in terms of
potential for make-work for mantainer once automated systems like
syzbot try to blindly use and it results in huge numbers of false
positive reports that we then have to work through as an unfunded
mandate.
If you want to add lock annotations into the struct page or even
struct folio, I cordially invite you to try running that by the mm
developers, who will probably tell you why that is a terrible idea
since it bloats a critical data structure.
Cheers,
- Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible circular dependency between i_data_sem and folio lock in ext4 filesystem
2024-07-11 15:38 ` Theodore Ts'o
@ 2024-07-12 4:44 ` Byungchul Park
2024-07-12 5:31 ` Byungchul Park
0 siblings, 1 reply; 6+ messages in thread
From: Byungchul Park @ 2024-07-12 4:44 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Hyeonggon Yoo, Linux Memory Management List, linux-fsdevel,
linux-ext4, max.byungchul.park, Gwan-gyeong Mun, kernel_team
On Thu, Jul 11, 2024 at 11:38:46AM -0400, Theodore Ts'o wrote:
> On Thu, Jul 11, 2024 at 09:07:53PM +0900, Hyeonggon Yoo wrote:
> > Hi folks,
> >
> > Byungchul, Gwan-gyeong and I are investigating possible circular
> > dependency reported by a dependency tracker named DEPT [1], which is
> > able to report possible circular dependencies involving folio locks
> > and other forms of dependencies that are not locks (i.e., wait for
> > completion).
> >
> > Below are two similar reports from DEPT where one context takes
> > i_data_sem and then folio lock in ext4_map_blocks(), while the other
> > context takes folio lock and then i_data_sem during processing of
> > pwrite64() system calls. We're reaching out due to a lack of
> > understanding of ext4 and file system internals.
> >
> > The points in question are:
> >
> > - Can the two contexts actually create a dependency between each other
> > in ext4? In other words, do their uses of folio lock make them belong
> > to the same lock classes?
>
> No.
>
> > - Are there any locking rules in ext4 that ensure these two contexts
> > will never be considered as the same lock class?
>
> It's inherent is the code path. In one of the stack traces, we are
> using the page cache for the bitmap allocation block (in other words, a metadata
> block). In the other stack trace, the page cache belongs to a regular
> file (in other words, a data block).
>
> So this is a false positive with DEPT, which has always been one of
> the reasons why I've been dubious about the value of DEPT in terms of
> potential for make-work for mantainer once automated systems like
> syzbot try to blindly use and it results in huge numbers of false
> positive reports that we then have to work through as an unfunded
> mandate.
What a funny guy... He did neither 1) insisting it's a bug in your code
nor 3) insisting DEPT is a great tool, but just asking if there's any
locking rules based on the *different acqusition order* between folio
lock and i_data_sem that he observed anyway.
I don't think you are a guy who introduces bugs, but the thing is it's
hard to find a *document* describing locking rules. Anyone could get
fairly curious about the different acquisition order. It's an open
source project. You are responsible for appropriate document as well.
I don't understand why you act to DEPT like that by the way. You don't
have to becasue:
1. I added the *EXPERIMENTAL* tag in Kconfig as you suggested, which
will prevent autotesting until it's considered stable. However,
the report from DEPT can be a good hint to someone.
2. DEPT can locate code where needs to be documented even if it's not
a real bug. It could even help better documentation.
DEPT hurts neither code nor performance unless enabling it.
> If you want to add lock annotations into the struct page or even
> struct folio, I cordially invite you to try running that by the mm
> developers, who will probably tell you why that is a terrible idea
> since it bloats a critical data structure.
I already said several times. Doesn't consume struct page.
Byungchul
> Cheers,
>
> - Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible circular dependency between i_data_sem and folio lock in ext4 filesystem
2024-07-12 4:44 ` Byungchul Park
@ 2024-07-12 5:31 ` Byungchul Park
2024-07-12 21:23 ` Vlastimil Babka (SUSE)
0 siblings, 1 reply; 6+ messages in thread
From: Byungchul Park @ 2024-07-12 5:31 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Hyeonggon Yoo, Linux Memory Management List, linux-fsdevel,
linux-ext4, max.byungchul.park, Gwan-gyeong Mun, kernel_team
On Fri, Jul 12, 2024 at 01:44:20PM +0900, Byungchul Park wrote:
> On Thu, Jul 11, 2024 at 11:38:46AM -0400, Theodore Ts'o wrote:
> > On Thu, Jul 11, 2024 at 09:07:53PM +0900, Hyeonggon Yoo wrote:
> > > Hi folks,
> > >
> > > Byungchul, Gwan-gyeong and I are investigating possible circular
> > > dependency reported by a dependency tracker named DEPT [1], which is
> > > able to report possible circular dependencies involving folio locks
> > > and other forms of dependencies that are not locks (i.e., wait for
> > > completion).
> > >
> > > Below are two similar reports from DEPT where one context takes
> > > i_data_sem and then folio lock in ext4_map_blocks(), while the other
> > > context takes folio lock and then i_data_sem during processing of
> > > pwrite64() system calls. We're reaching out due to a lack of
> > > understanding of ext4 and file system internals.
> > >
> > > The points in question are:
> > >
> > > - Can the two contexts actually create a dependency between each other
> > > in ext4? In other words, do their uses of folio lock make them belong
> > > to the same lock classes?
> >
> > No.
> >
> > > - Are there any locking rules in ext4 that ensure these two contexts
> > > will never be considered as the same lock class?
> >
> > It's inherent is the code path. In one of the stack traces, we are
> > using the page cache for the bitmap allocation block (in other words, a metadata
> > block). In the other stack trace, the page cache belongs to a regular
> > file (in other words, a data block).
> >
> > So this is a false positive with DEPT, which has always been one of
> > the reasons why I've been dubious about the value of DEPT in terms of
> > potential for make-work for mantainer once automated systems like
> > syzbot try to blindly use and it results in huge numbers of false
> > positive reports that we then have to work through as an unfunded
> > mandate.
>
> What a funny guy... He did neither 1) insisting it's a bug in your code
> nor 3) insisting DEPT is a great tool, but just asking if there's any
> locking rules based on the *different acqusition order* between folio
> lock and i_data_sem that he observed anyway.
>
> I don't think you are a guy who introduces bugs, but the thing is it's
> hard to find a *document* describing locking rules. Anyone could get
> fairly curious about the different acquisition order. It's an open
> source project. You are responsible for appropriate document as well.
>
> I don't understand why you act to DEPT like that by the way. You don't
> have to becasue:
>
> 1. I added the *EXPERIMENTAL* tag in Kconfig as you suggested, which
> will prevent autotesting until it's considered stable. However,
> the report from DEPT can be a good hint to someone.
>
> 2. DEPT can locate code where needs to be documented even if it's not
> a real bug. It could even help better documentation.
>
> DEPT hurts neither code nor performance unless enabling it.
>
> > If you want to add lock annotations into the struct page or even
> > struct folio, I cordially invite you to try running that by the mm
> > developers, who will probably tell you why that is a terrible idea
> > since it bloats a critical data structure.
>
> I already said several times. Doesn't consume struct page.
Sorry for that. I've changed the code so the current version consumes
it by about two words if enabled. I can place it to page_ext as before
if needed.
Byungchul
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible circular dependency between i_data_sem and folio lock in ext4 filesystem
2024-07-12 5:31 ` Byungchul Park
@ 2024-07-12 21:23 ` Vlastimil Babka (SUSE)
2024-07-15 10:32 ` Byungchul Park
0 siblings, 1 reply; 6+ messages in thread
From: Vlastimil Babka (SUSE) @ 2024-07-12 21:23 UTC (permalink / raw)
To: Byungchul Park, Theodore Ts'o
Cc: Hyeonggon Yoo, Linux Memory Management List, linux-fsdevel,
linux-ext4, max.byungchul.park, Gwan-gyeong Mun, kernel_team
On 7/12/24 7:31 AM, Byungchul Park wrote:
> On Fri, Jul 12, 2024 at 01:44:20PM +0900, Byungchul Park wrote:
>>
>> What a funny guy... He did neither 1) insisting it's a bug in your code
>> nor 3) insisting DEPT is a great tool, but just asking if there's any
>> locking rules based on the *different acqusition order* between folio
>> lock and i_data_sem that he observed anyway.
>>
>> I don't think you are a guy who introduces bugs, but the thing is it's
>> hard to find a *document* describing locking rules. Anyone could get
>> fairly curious about the different acquisition order. It's an open
>> source project. You are responsible for appropriate document as well.
>>
>> I don't understand why you act to DEPT like that by the way. You don't
>> have to becasue:
>>
>> 1. I added the *EXPERIMENTAL* tag in Kconfig as you suggested, which
>> will prevent autotesting until it's considered stable. However,
>> the report from DEPT can be a good hint to someone.
>>
>> 2. DEPT can locate code where needs to be documented even if it's not
>> a real bug. It could even help better documentation.
>>
>> DEPT hurts neither code nor performance unless enabling it.
enabling means building with CONFIG_DEPT right?
>> > If you want to add lock annotations into the struct page or even
>> > struct folio, I cordially invite you to try running that by the mm
>> > developers, who will probably tell you why that is a terrible idea
>> > since it bloats a critical data structure.
I doubt anyone will object making struct page larger for a non-production
debugging config option, which AFAIU DEPT is, i.e. in the same area as
LOCKDEP or KASAN etc... I can see at least KMSAN already adds some fields to
struct page already.
>> I already said several times. Doesn't consume struct page.
>
> Sorry for that. I've changed the code so the current version consumes
> it by about two words if enabled. I can place it to page_ext as before
> if needed.
page_ext is useful if you have a debugging feature that can be compiled in
but adds no overhead (memory, nor cpu thanks to static keys) unless enabled
on boot time, i.e. page_owner... so for DEPT it seems it would be an
unnecessary complication.
> Byungchul
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible circular dependency between i_data_sem and folio lock in ext4 filesystem
2024-07-12 21:23 ` Vlastimil Babka (SUSE)
@ 2024-07-15 10:32 ` Byungchul Park
0 siblings, 0 replies; 6+ messages in thread
From: Byungchul Park @ 2024-07-15 10:32 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: Theodore Ts'o, Hyeonggon Yoo, Linux Memory Management List,
linux-fsdevel, linux-ext4, max.byungchul.park, Gwan-gyeong Mun,
kernel_team
On Fri, Jul 12, 2024 at 11:23:36PM +0200, Vlastimil Babka (SUSE) wrote:
> On 7/12/24 7:31 AM, Byungchul Park wrote:
> > On Fri, Jul 12, 2024 at 01:44:20PM +0900, Byungchul Park wrote:
> >>
> >> What a funny guy... He did neither 1) insisting it's a bug in your code
> >> nor 3) insisting DEPT is a great tool, but just asking if there's any
> >> locking rules based on the *different acqusition order* between folio
> >> lock and i_data_sem that he observed anyway.
> >>
> >> I don't think you are a guy who introduces bugs, but the thing is it's
> >> hard to find a *document* describing locking rules. Anyone could get
> >> fairly curious about the different acquisition order. It's an open
> >> source project. You are responsible for appropriate document as well.
> >>
> >> I don't understand why you act to DEPT like that by the way. You don't
> >> have to becasue:
> >>
> >> 1. I added the *EXPERIMENTAL* tag in Kconfig as you suggested, which
> >> will prevent autotesting until it's considered stable. However,
> >> the report from DEPT can be a good hint to someone.
> >>
> >> 2. DEPT can locate code where needs to be documented even if it's not
> >> a real bug. It could even help better documentation.
> >>
> >> DEPT hurts neither code nor performance unless enabling it.
>
> enabling means building with CONFIG_DEPT right?
Yes.
> >> > If you want to add lock annotations into the struct page or even
> >> > struct folio, I cordially invite you to try running that by the mm
> >> > developers, who will probably tell you why that is a terrible idea
> >> > since it bloats a critical data structure.
>
> I doubt anyone will object making struct page larger for a non-production
> debugging config option, which AFAIU DEPT is, i.e. in the same area as
> LOCKDEP or KASAN etc... I can see at least KMSAN already adds some fields to
> struct page already.
I think so.
> >> I already said several times. Doesn't consume struct page.
> >
> > Sorry for that. I've changed the code so the current version consumes
> > it by about two words if enabled. I can place it to page_ext as before
> > if needed.
>
> page_ext is useful if you have a debugging feature that can be compiled in
> but adds no overhead (memory, nor cpu thanks to static keys) unless enabled
> on boot time, i.e. page_owner... so for DEPT it seems it would be an
> unnecessary complication.
Yeah, I will think it more. However, maybe, as you said, it could
introduce a complication. Thanks.
Byungchul
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-15 10:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-11 12:07 Possible circular dependency between i_data_sem and folio lock in ext4 filesystem Hyeonggon Yoo
2024-07-11 15:38 ` Theodore Ts'o
2024-07-12 4:44 ` Byungchul Park
2024-07-12 5:31 ` Byungchul Park
2024-07-12 21:23 ` Vlastimil Babka (SUSE)
2024-07-15 10:32 ` Byungchul Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).