All of lore.kernel.org
 help / color / mirror / Atom feed
* linux-next test error
@ 2018-09-05  7:13 syzbot
  2018-09-05  7:17 ` Dmitry Vyukov
  2018-09-05  8:55 ` Jan Kara
  0 siblings, 2 replies; 19+ messages in thread
From: syzbot @ 2018-09-05  7:13 UTC (permalink / raw)
  To: ak, akpm, jack, jrdr.linux, linux-kernel, linux-mm, mawilcox,
	mgorman, syzkaller-bugs, tim.c.chen, zwisler

Hello,

syzbot found the following crash on:

HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
git tree:       linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com

INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for  
more than 140 seconds.
       Not tainted 4.19.0-rc2-next-20180905+ #56
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-fuzzer      D21704  4876   4871 0x00000000
Call Trace:
  context_switch kernel/sched/core.c:2825 [inline]
  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
  schedule+0xfb/0x450 kernel/sched/core.c:3517
  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
  wait_on_page_bit_common mm/filemap.c:1100 [inline]
  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
  lock_page include/linux/pagemap.h:483 [inline]
  do_page_mkwrite+0x429/0x520 mm/memory.c:2391
  do_shared_fault mm/memory.c:3717 [inline]
  do_fault mm/memory.c:3756 [inline]
  handle_pte_fault mm/memory.c:3983 [inline]
  __handle_mm_fault+0x2a0a/0x4350 mm/memory.c:4107
  handle_mm_fault+0x53e/0xc80 mm/memory.c:4144
  __do_page_fault+0x620/0xe50 arch/x86/mm/fault.c:1395
  do_page_fault+0xf6/0x7a4 arch/x86/mm/fault.c:1470
  page_fault+0x1e/0x30 arch/x86/entry/entry_64.S:1164
RIP: 0033:0x70b591
Code: 48 89 6c 24 10 48 8d 6c 24 10 48 8b 44 24 20 48 8b 48 08 48 8b 50 10  
48 8d 58 08 48 83 fa 08 0f 8c 1e 01 00 00 48 8b 54 24 28 <88> 11 48 8b 48  
08 48 8b 70 10 48 83 fe 01 0f 86 44 01 00 00 48 89
RSP: 002b:000000c42010f3f0 EFLAGS: 00010212
RAX: 000000c42010f508 RBX: 000000c42010f510 RCX: 00007f9d22c27000
RDX: 0000000000000537 RSI: 000000000072a470 RDI: 000000c42010f348
RBP: 000000c42010f400 R08: 0000000000000000 R09: 0000000000000000
R10: 00000000009496ca R11: 0000000000000004 R12: 0000000000000000
R13: 0000000000000020 R14: 0000000000000013 R15: 000000000000000f

Showing all locks held in the system:
4 locks held by kworker/u4:1/22:
  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:  
__write_once_size include/linux/compiler.h:215 [inline]
  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:  
arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at: atomic64_set  
include/asm-generic/atomic-instrumented.h:40 [inline]
  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:  
atomic_long_set include/asm-generic/atomic-long.h:59 [inline]
  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at: set_work_data  
kernel/workqueue.c:617 [inline]
  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:  
set_work_pool_and_clear_pending kernel/workqueue.c:644 [inline]
  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:  
process_one_work+0xb44/0x1aa0 kernel/workqueue.c:2124
  #1: 000000007193e1ae ((work_completion)(&(&wb->dwork)->work)){+.+.}, at:  
process_one_work+0xb9b/0x1aa0 kernel/workqueue.c:2128
  #2: 0000000079a3ee6d (&fc->fs_type->s_umount_key#29){++++}, at:  
trylock_super+0x22/0x110 fs/super.c:411
  #3: 00000000f58a212d (&sbi->s_journal_flag_rwsem){.+.+}, at:  
do_writepages+0x9a/0x1a0 mm/page-writeback.c:2340
1 lock held by khungtaskd/793:
  #0: 0000000020ca7c68 (rcu_read_lock){....}, at:  
debug_show_all_locks+0xd0/0x428 kernel/locking/lockdep.c:4436
1 lock held by khugepaged/799:
  #0: 00000000af3da9ce (&mm->mmap_sem){++++}, at:  
collapse_huge_page+0x2bf/0x2250 mm/khugepaged.c:1007
1 lock held by rsyslogd/4761:
  #0: 00000000a67fe71d (&f->f_pos_lock){+.+.}, at: __fdget_pos+0x1bb/0x200  
fs/file.c:766
2 locks held by getty/4851:
  #0: 00000000236449d0 (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
  #1: 000000004a997762 (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4852:
  #0: 00000000d68d1a08 (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
  #1: 00000000f062cc2f (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4853:
  #0: 00000000507cd5fe (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
  #1: 00000000fa59d11e (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4854:
  #0: 00000000de7b4c24 (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
  #1: 00000000e3caa2a6 (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4855:
  #0: 00000000e5b05f9f (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
  #1: 0000000002e17b37 (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4856:
  #0: 00000000831fbb62 (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
  #1: 000000009838e2ef (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4857:
  #0: 000000003038a645 (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
  #1: 00000000c1269869 (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
1 lock held by syz-fuzzer/4876:
  #0: 00000000af3da9ce (&mm->mmap_sem){++++}, at:  
__do_page_fault+0x389/0xe50 arch/x86/mm/fault.c:1324

=============================================

NMI backtrace for cpu 0
CPU: 0 PID: 793 Comm: khungtaskd Not tainted 4.19.0-rc2-next-20180905+ #56
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
  nmi_cpu_backtrace.cold.4+0x48/0x88 lib/nmi_backtrace.c:101
  nmi_trigger_cpumask_backtrace+0x1b6/0x1cd lib/nmi_backtrace.c:62
  arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
  trigger_all_cpu_backtrace include/linux/nmi.h:144 [inline]
  check_hung_uninterruptible_tasks kernel/hung_task.c:204 [inline]
  watchdog+0xb39/0x1040 kernel/hung_task.c:265
  kthread+0x35a/0x420 kernel/kthread.c:246
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:415
Sending NMI from CPU 0 to CPUs 1:
NMI backtrace for cpu 1
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.19.0-rc2-next-20180905+ #56
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:__read_once_size include/linux/compiler.h:188 [inline]
RIP: 0010:check_kcov_mode kernel/kcov.c:69 [inline]
RIP: 0010:write_comp_data+0x22/0x70 kernel/kcov.c:122
Code: e4 b9 ca ff 90 90 90 90 55 65 4c 8b 04 25 40 ee 01 00 65 8b 05 2f 44  
86 7e 48 89 e5 a9 00 01 1f 00 75 51 41 8b 80 88 12 00 00 <83> f8 03 75 45  
49 8b 80 90 12 00 00 45 8b 80 8c 12 00 00 4c 8b 08
RSP: 0018:ffff8801d9f1fb10 EFLAGS: 00000046
RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff816df180
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000001
RBP: ffff8801d9f1fb10 R08: ffff8801d9f0e380 R09: 0000000000000000
R10: 0000000000000003 R11: 0000000000000000 R12: ffff8801d9f1fb80
R13: 0000000000021f3e R14: 0000002b4d7800be R15: 00000000fffe3d07
FS:  0000000000000000(0000) GS:ffff8801db100000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffff600400 CR3: 00000001b91c2000 CR4: 00000000001406e0
Call Trace:
  __sanitizer_cov_trace_const_cmp1+0x1a/0x20 kernel/kcov.c:174
  tick_nohz_next_event+0x5e0/0x8a0 kernel/time/tick-sched.c:672
  __tick_nohz_idle_stop_tick kernel/time/tick-sched.c:930 [inline]
  tick_nohz_idle_stop_tick+0x633/0xcb0 kernel/time/tick-sched.c:960
  cpuidle_idle_call kernel/sched/idle.c:150 [inline]
  do_idle+0x3a0/0x580 kernel/sched/idle.c:262
  cpu_startup_entry+0x10c/0x120 kernel/sched/idle.c:368
  start_secondary+0x433/0x5d0 arch/x86/kernel/smpboot.c:271
  secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:242


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05  7:13 linux-next test error syzbot
@ 2018-09-05  7:17 ` Dmitry Vyukov
  2018-09-05  8:55 ` Jan Kara
  1 sibling, 0 replies; 19+ messages in thread
From: Dmitry Vyukov @ 2018-09-05  7:17 UTC (permalink / raw)
  To: syzbot
  Cc: Andi Kleen, Andrew Morton, Jan Kara, Souptick Joarder, LKML,
	Linux-MM, Matthew Wilcox, Mel Gorman, syzkaller-bugs, tim.c.chen,
	zwisler, Theodore Ts'o

On Wed, Sep 5, 2018 at 9:13 AM, syzbot
<syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
> git tree:       linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
> dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com

This happens during simple image testing, mostly create a process that
does mmap. No fuzzing involved yet.

This happened around the same time I enabled extents feature in syzbot
ext4 images:
https://github.com/google/syzkaller/commit/98bfd6d34cf1dfe8b9638f88017f331e06325c25
Maybe it's related, but the hang does not happen on any other trees so far.


> INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for
> more than 140 seconds.
>       Not tainted 4.19.0-rc2-next-20180905+ #56
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> syz-fuzzer      D21704  4876   4871 0x00000000
> Call Trace:
>  context_switch kernel/sched/core.c:2825 [inline]
>  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
>  schedule+0xfb/0x450 kernel/sched/core.c:3517
>  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
>  wait_on_page_bit_common mm/filemap.c:1100 [inline]
>  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
>  lock_page include/linux/pagemap.h:483 [inline]
>  do_page_mkwrite+0x429/0x520 mm/memory.c:2391
>  do_shared_fault mm/memory.c:3717 [inline]
>  do_fault mm/memory.c:3756 [inline]
>  handle_pte_fault mm/memory.c:3983 [inline]
>  __handle_mm_fault+0x2a0a/0x4350 mm/memory.c:4107
>  handle_mm_fault+0x53e/0xc80 mm/memory.c:4144
>  __do_page_fault+0x620/0xe50 arch/x86/mm/fault.c:1395
>  do_page_fault+0xf6/0x7a4 arch/x86/mm/fault.c:1470
>  page_fault+0x1e/0x30 arch/x86/entry/entry_64.S:1164
> RIP: 0033:0x70b591
> Code: 48 89 6c 24 10 48 8d 6c 24 10 48 8b 44 24 20 48 8b 48 08 48 8b 50 10
> 48 8d 58 08 48 83 fa 08 0f 8c 1e 01 00 00 48 8b 54 24 28 <88> 11 48 8b 48 08
> 48 8b 70 10 48 83 fe 01 0f 86 44 01 00 00 48 89
> RSP: 002b:000000c42010f3f0 EFLAGS: 00010212
> RAX: 000000c42010f508 RBX: 000000c42010f510 RCX: 00007f9d22c27000
> RDX: 0000000000000537 RSI: 000000000072a470 RDI: 000000c42010f348
> RBP: 000000c42010f400 R08: 0000000000000000 R09: 0000000000000000
> R10: 00000000009496ca R11: 0000000000000004 R12: 0000000000000000
> R13: 0000000000000020 R14: 0000000000000013 R15: 000000000000000f
>
> Showing all locks held in the system:
> 4 locks held by kworker/u4:1/22:
>  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:
> __write_once_size include/linux/compiler.h:215 [inline]
>  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:
> arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
>  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at: atomic64_set
> include/asm-generic/atomic-instrumented.h:40 [inline]
>  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:
> atomic_long_set include/asm-generic/atomic-long.h:59 [inline]
>  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at: set_work_data
> kernel/workqueue.c:617 [inline]
>  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:
> set_work_pool_and_clear_pending kernel/workqueue.c:644 [inline]
>  #0: 000000005bc28536 ((wq_completion)"writeback"){+.+.}, at:
> process_one_work+0xb44/0x1aa0 kernel/workqueue.c:2124
>  #1: 000000007193e1ae ((work_completion)(&(&wb->dwork)->work)){+.+.}, at:
> process_one_work+0xb9b/0x1aa0 kernel/workqueue.c:2128
>  #2: 0000000079a3ee6d (&fc->fs_type->s_umount_key#29){++++}, at:
> trylock_super+0x22/0x110 fs/super.c:411
>  #3: 00000000f58a212d (&sbi->s_journal_flag_rwsem){.+.+}, at:
> do_writepages+0x9a/0x1a0 mm/page-writeback.c:2340
> 1 lock held by khungtaskd/793:
>  #0: 0000000020ca7c68 (rcu_read_lock){....}, at:
> debug_show_all_locks+0xd0/0x428 kernel/locking/lockdep.c:4436
> 1 lock held by khugepaged/799:
>  #0: 00000000af3da9ce (&mm->mmap_sem){++++}, at:
> collapse_huge_page+0x2bf/0x2250 mm/khugepaged.c:1007
> 1 lock held by rsyslogd/4761:
>  #0: 00000000a67fe71d (&f->f_pos_lock){+.+.}, at: __fdget_pos+0x1bb/0x200
> fs/file.c:766
> 2 locks held by getty/4851:
>  #0: 00000000236449d0 (&tty->ldisc_sem){++++}, at: ldsem_down_read+0x37/0x40
> drivers/tty/tty_ldsem.c:353
>  #1: 000000004a997762 (&ldata->atomic_read_lock){+.+.}, at:
> n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
> 2 locks held by getty/4852:
>  #0: 00000000d68d1a08 (&tty->ldisc_sem){++++}, at: ldsem_down_read+0x37/0x40
> drivers/tty/tty_ldsem.c:353
>  #1: 00000000f062cc2f (&ldata->atomic_read_lock){+.+.}, at:
> n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
> 2 locks held by getty/4853:
>  #0: 00000000507cd5fe (&tty->ldisc_sem){++++}, at: ldsem_down_read+0x37/0x40
> drivers/tty/tty_ldsem.c:353
>  #1: 00000000fa59d11e (&ldata->atomic_read_lock){+.+.}, at:
> n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
> 2 locks held by getty/4854:
>  #0: 00000000de7b4c24 (&tty->ldisc_sem){++++}, at: ldsem_down_read+0x37/0x40
> drivers/tty/tty_ldsem.c:353
>  #1: 00000000e3caa2a6 (&ldata->atomic_read_lock){+.+.}, at:
> n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
> 2 locks held by getty/4855:
>  #0: 00000000e5b05f9f (&tty->ldisc_sem){++++}, at: ldsem_down_read+0x37/0x40
> drivers/tty/tty_ldsem.c:353
>  #1: 0000000002e17b37 (&ldata->atomic_read_lock){+.+.}, at:
> n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
> 2 locks held by getty/4856:
>  #0: 00000000831fbb62 (&tty->ldisc_sem){++++}, at: ldsem_down_read+0x37/0x40
> drivers/tty/tty_ldsem.c:353
>  #1: 000000009838e2ef (&ldata->atomic_read_lock){+.+.}, at:
> n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
> 2 locks held by getty/4857:
>  #0: 000000003038a645 (&tty->ldisc_sem){++++}, at: ldsem_down_read+0x37/0x40
> drivers/tty/tty_ldsem.c:353
>  #1: 00000000c1269869 (&ldata->atomic_read_lock){+.+.}, at:
> n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
> 1 lock held by syz-fuzzer/4876:
>  #0: 00000000af3da9ce (&mm->mmap_sem){++++}, at: __do_page_fault+0x389/0xe50
> arch/x86/mm/fault.c:1324
>
> =============================================
>
> NMI backtrace for cpu 0
> CPU: 0 PID: 793 Comm: khungtaskd Not tainted 4.19.0-rc2-next-20180905+ #56
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
>  nmi_cpu_backtrace.cold.4+0x48/0x88 lib/nmi_backtrace.c:101
>  nmi_trigger_cpumask_backtrace+0x1b6/0x1cd lib/nmi_backtrace.c:62
>  arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
>  trigger_all_cpu_backtrace include/linux/nmi.h:144 [inline]
>  check_hung_uninterruptible_tasks kernel/hung_task.c:204 [inline]
>  watchdog+0xb39/0x1040 kernel/hung_task.c:265
>  kthread+0x35a/0x420 kernel/kthread.c:246
>  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:415
> Sending NMI from CPU 0 to CPUs 1:
> NMI backtrace for cpu 1
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.19.0-rc2-next-20180905+ #56
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:__read_once_size include/linux/compiler.h:188 [inline]
> RIP: 0010:check_kcov_mode kernel/kcov.c:69 [inline]
> RIP: 0010:write_comp_data+0x22/0x70 kernel/kcov.c:122
> Code: e4 b9 ca ff 90 90 90 90 55 65 4c 8b 04 25 40 ee 01 00 65 8b 05 2f 44
> 86 7e 48 89 e5 a9 00 01 1f 00 75 51 41 8b 80 88 12 00 00 <83> f8 03 75 45 49
> 8b 80 90 12 00 00 45 8b 80 8c 12 00 00 4c 8b 08
> RSP: 0018:ffff8801d9f1fb10 EFLAGS: 00000046
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff816df180
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000001
> RBP: ffff8801d9f1fb10 R08: ffff8801d9f0e380 R09: 0000000000000000
> R10: 0000000000000003 R11: 0000000000000000 R12: ffff8801d9f1fb80
> R13: 0000000000021f3e R14: 0000002b4d7800be R15: 00000000fffe3d07
> FS:  0000000000000000(0000) GS:ffff8801db100000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffff600400 CR3: 00000001b91c2000 CR4: 00000000001406e0
> Call Trace:
>  __sanitizer_cov_trace_const_cmp1+0x1a/0x20 kernel/kcov.c:174
>  tick_nohz_next_event+0x5e0/0x8a0 kernel/time/tick-sched.c:672
>  __tick_nohz_idle_stop_tick kernel/time/tick-sched.c:930 [inline]
>  tick_nohz_idle_stop_tick+0x633/0xcb0 kernel/time/tick-sched.c:960
>  cpuidle_idle_call kernel/sched/idle.c:150 [inline]
>  do_idle+0x3a0/0x580 kernel/sched/idle.c:262
>  cpu_startup_entry+0x10c/0x120 kernel/sched/idle.c:368
>  start_secondary+0x433/0x5d0 arch/x86/kernel/smpboot.c:271
>  secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:242
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/syzkaller-bugs/0000000000004f6b5805751a8189%40google.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05  7:13 linux-next test error syzbot
  2018-09-05  7:17 ` Dmitry Vyukov
@ 2018-09-05  8:55 ` Jan Kara
  2018-09-05  9:50   ` Souptick Joarder
  2018-09-05 19:07   ` Souptick Joarder
  1 sibling, 2 replies; 19+ messages in thread
From: Jan Kara @ 2018-09-05  8:55 UTC (permalink / raw)
  To: syzbot
  Cc: ak, akpm, jack, jrdr.linux, linux-kernel, linux-mm, mawilcox,
	mgorman, syzkaller-bugs, tim.c.chen, zwisler

On Wed 05-09-18 00:13:02, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
> git tree:       linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
> dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> 
> Unfortunately, I don't have any reproducer for this crash yet.
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com
> 
> INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for
> more than 140 seconds.
>       Not tainted 4.19.0-rc2-next-20180905+ #56
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> syz-fuzzer      D21704  4876   4871 0x00000000
> Call Trace:
>  context_switch kernel/sched/core.c:2825 [inline]
>  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
>  schedule+0xfb/0x450 kernel/sched/core.c:3517
>  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
>  wait_on_page_bit_common mm/filemap.c:1100 [inline]
>  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
>  lock_page include/linux/pagemap.h:483 [inline]
>  do_page_mkwrite+0x429/0x520 mm/memory.c:2391

Waiting for page lock after ->page_mkwrite callback. Which means
->page_mkwrite did not return VM_FAULT_LOCKED but 0. Looking into
linux-next... indeed "fs: convert return type int to vm_fault_t" has busted
block_page_mkwrite(). It has to return VM_FAULT_LOCKED and not 0 now.
Souptick, can I ask you to run 'fstests' for at least common filesystems
like ext4, xfs, btrfs when you change generic filesystem code please? That
would catch a bug like this immediately. Thanks.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05  8:55 ` Jan Kara
@ 2018-09-05  9:50   ` Souptick Joarder
  2018-09-05  9:53     ` Dmitry Vyukov
                       ` (2 more replies)
  2018-09-05 19:07   ` Souptick Joarder
  1 sibling, 3 replies; 19+ messages in thread
From: Souptick Joarder @ 2018-09-05  9:50 UTC (permalink / raw)
  To: Jan Kara
  Cc: syzbot+87a05ae4accd500f5242, ak, Andrew Morton, linux-kernel,
	Linux-MM, mawilcox, mgorman, syzkaller-bugs, tim.c.chen, zwisler

On Wed, Sep 5, 2018 at 2:25 PM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 05-09-18 00:13:02, syzbot wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
> > git tree:       linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
> > dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
> > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> >
> > Unfortunately, I don't have any reproducer for this crash yet.
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com
> >
> > INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for
> > more than 140 seconds.
> >       Not tainted 4.19.0-rc2-next-20180905+ #56
> > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > syz-fuzzer      D21704  4876   4871 0x00000000
> > Call Trace:
> >  context_switch kernel/sched/core.c:2825 [inline]
> >  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
> >  schedule+0xfb/0x450 kernel/sched/core.c:3517
> >  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
> >  wait_on_page_bit_common mm/filemap.c:1100 [inline]
> >  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
> >  lock_page include/linux/pagemap.h:483 [inline]
> >  do_page_mkwrite+0x429/0x520 mm/memory.c:2391
>
> Waiting for page lock after ->page_mkwrite callback. Which means
> ->page_mkwrite did not return VM_FAULT_LOCKED but 0. Looking into
> linux-next... indeed "fs: convert return type int to vm_fault_t" has busted
> block_page_mkwrite(). It has to return VM_FAULT_LOCKED and not 0 now.
> Souptick, can I ask you to run 'fstests' for at least common filesystems
> like ext4, xfs, btrfs when you change generic filesystem code please? That
> would catch a bug like this immediately. Thanks.
>

"fs: convert return type int to vm_fault_t" is still under
review/discusson and not yet merge
into linux-next. I am not seeing it into linux-next tree.Can you
please share the commit id ?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05  9:50   ` Souptick Joarder
@ 2018-09-05  9:53     ` Dmitry Vyukov
  2018-09-05  9:57       ` Dmitry Vyukov
  2018-09-05 13:34     ` Theodore Y. Ts'o
  2018-09-05 13:55     ` Jan Kara
  2 siblings, 1 reply; 19+ messages in thread
From: Dmitry Vyukov @ 2018-09-05  9:53 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Jan Kara, syzbot, Andi Kleen, Andrew Morton, LKML, Linux-MM,
	Matthew Wilcox, Mel Gorman, syzkaller-bugs, tim.c.chen, zwisler

On Wed, Sep 5, 2018 at 11:50 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
> On Wed, Sep 5, 2018 at 2:25 PM Jan Kara <jack@suse.cz> wrote:
>>
>> On Wed 05-09-18 00:13:02, syzbot wrote:
>> > Hello,
>> >
>> > syzbot found the following crash on:
>> >
>> > HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
>> > git tree:       linux-next
>> > console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
>> > kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
>> > dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
>> > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>> >
>> > Unfortunately, I don't have any reproducer for this crash yet.
>> >
>> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> > Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com
>> >
>> > INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for
>> > more than 140 seconds.
>> >       Not tainted 4.19.0-rc2-next-20180905+ #56
>> > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> > syz-fuzzer      D21704  4876   4871 0x00000000
>> > Call Trace:
>> >  context_switch kernel/sched/core.c:2825 [inline]
>> >  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
>> >  schedule+0xfb/0x450 kernel/sched/core.c:3517
>> >  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
>> >  wait_on_page_bit_common mm/filemap.c:1100 [inline]
>> >  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
>> >  lock_page include/linux/pagemap.h:483 [inline]
>> >  do_page_mkwrite+0x429/0x520 mm/memory.c:2391
>>
>> Waiting for page lock after ->page_mkwrite callback. Which means
>> ->page_mkwrite did not return VM_FAULT_LOCKED but 0. Looking into
>> linux-next... indeed "fs: convert return type int to vm_fault_t" has busted
>> block_page_mkwrite(). It has to return VM_FAULT_LOCKED and not 0 now.
>> Souptick, can I ask you to run 'fstests' for at least common filesystems
>> like ext4, xfs, btrfs when you change generic filesystem code please? That
>> would catch a bug like this immediately. Thanks.
>>
>
> "fs: convert return type int to vm_fault_t" is still under
> review/discusson and not yet merge
> into linux-next. I am not seeing it into linux-next tree.Can you
> please share the commit id ?

syzbot gave commit id in the report (see above)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05  9:53     ` Dmitry Vyukov
@ 2018-09-05  9:57       ` Dmitry Vyukov
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Vyukov @ 2018-09-05  9:57 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Jan Kara, syzbot, Andi Kleen, Andrew Morton, LKML, Linux-MM,
	Mel Gorman, syzkaller-bugs, tim.c.chen, zwisler

On Wed, Sep 5, 2018 at 11:53 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Wed, Sep 5, 2018 at 11:50 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
>> On Wed, Sep 5, 2018 at 2:25 PM Jan Kara <jack@suse.cz> wrote:
>>>
>>> On Wed 05-09-18 00:13:02, syzbot wrote:
>>> > Hello,
>>> >
>>> > syzbot found the following crash on:
>>> >
>>> > HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
>>> > git tree:       linux-next
>>> > console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
>>> > kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
>>> > dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
>>> > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>>> >
>>> > Unfortunately, I don't have any reproducer for this crash yet.
>>> >
>>> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> > Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com
>>> >
>>> > INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for
>>> > more than 140 seconds.
>>> >       Not tainted 4.19.0-rc2-next-20180905+ #56
>>> > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>>> > syz-fuzzer      D21704  4876   4871 0x00000000
>>> > Call Trace:
>>> >  context_switch kernel/sched/core.c:2825 [inline]
>>> >  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
>>> >  schedule+0xfb/0x450 kernel/sched/core.c:3517
>>> >  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
>>> >  wait_on_page_bit_common mm/filemap.c:1100 [inline]
>>> >  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
>>> >  lock_page include/linux/pagemap.h:483 [inline]
>>> >  do_page_mkwrite+0x429/0x520 mm/memory.c:2391
>>>
>>> Waiting for page lock after ->page_mkwrite callback. Which means
>>> ->page_mkwrite did not return VM_FAULT_LOCKED but 0. Looking into
>>> linux-next... indeed "fs: convert return type int to vm_fault_t" has busted
>>> block_page_mkwrite(). It has to return VM_FAULT_LOCKED and not 0 now.
>>> Souptick, can I ask you to run 'fstests' for at least common filesystems
>>> like ext4, xfs, btrfs when you change generic filesystem code please? That
>>> would catch a bug like this immediately. Thanks.
>>>
>>
>> "fs: convert return type int to vm_fault_t" is still under
>> review/discusson and not yet merge
>> into linux-next. I am not seeing it into linux-next tree.Can you
>> please share the commit id ?
>
> syzbot gave commit id in the report (see above)

-mawilcox@microsoft.com as I getting bounces from mail server

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05  9:50   ` Souptick Joarder
  2018-09-05  9:53     ` Dmitry Vyukov
@ 2018-09-05 13:34     ` Theodore Y. Ts'o
  2018-09-05 18:44       ` Christoph Hellwig
  2018-09-05 19:24       ` Souptick Joarder
  2018-09-05 13:55     ` Jan Kara
  2 siblings, 2 replies; 19+ messages in thread
From: Theodore Y. Ts'o @ 2018-09-05 13:34 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Jan Kara, syzbot+87a05ae4accd500f5242, ak, Andrew Morton,
	linux-kernel, Linux-MM, mawilcox, mgorman, syzkaller-bugs,
	tim.c.chen, zwisler, willy

On Wed, Sep 05, 2018 at 03:20:16PM +0530, Souptick Joarder wrote:
> 
> "fs: convert return type int to vm_fault_t" is still under
> review/discusson and not yet merge
> into linux-next. I am not seeing it into linux-next tree.Can you
> please share the commit id ?

It's at: 83c0adddcc6ed128168e7b87eaed0c21eac908e4 in the Linux Next
branch.

Dmitry, can you try reverting this commit and see if it makes the
problem go away?

Souptick, can we just NACK this patch and completely drop it from all
trees?

I think we need to be a *lot* more careful about this vm_fault_t patch
thing.  If you can't be bothered to run xfstests, we need to introduce
a new function which replaces block_page_mkwrite() --- and then let
each file system try to convert over to it at their own pace, after
they've done regression testing.

						- Ted

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05  9:50   ` Souptick Joarder
  2018-09-05  9:53     ` Dmitry Vyukov
  2018-09-05 13:34     ` Theodore Y. Ts'o
@ 2018-09-05 13:55     ` Jan Kara
  2 siblings, 0 replies; 19+ messages in thread
From: Jan Kara @ 2018-09-05 13:55 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Jan Kara, syzbot+87a05ae4accd500f5242, ak, Andrew Morton,
	linux-kernel, Linux-MM, mawilcox, mgorman, syzkaller-bugs,
	tim.c.chen, zwisler

On Wed 05-09-18 15:20:16, Souptick Joarder wrote:
> On Wed, Sep 5, 2018 at 2:25 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Wed 05-09-18 00:13:02, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
> > > git tree:       linux-next
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
> > > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> > >
> > > Unfortunately, I don't have any reproducer for this crash yet.
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com
> > >
> > > INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for
> > > more than 140 seconds.
> > >       Not tainted 4.19.0-rc2-next-20180905+ #56
> > > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > > syz-fuzzer      D21704  4876   4871 0x00000000
> > > Call Trace:
> > >  context_switch kernel/sched/core.c:2825 [inline]
> > >  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
> > >  schedule+0xfb/0x450 kernel/sched/core.c:3517
> > >  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
> > >  wait_on_page_bit_common mm/filemap.c:1100 [inline]
> > >  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
> > >  lock_page include/linux/pagemap.h:483 [inline]
> > >  do_page_mkwrite+0x429/0x520 mm/memory.c:2391
> >
> > Waiting for page lock after ->page_mkwrite callback. Which means
> > ->page_mkwrite did not return VM_FAULT_LOCKED but 0. Looking into
> > linux-next... indeed "fs: convert return type int to vm_fault_t" has busted
> > block_page_mkwrite(). It has to return VM_FAULT_LOCKED and not 0 now.
> > Souptick, can I ask you to run 'fstests' for at least common filesystems
> > like ext4, xfs, btrfs when you change generic filesystem code please? That
> > would catch a bug like this immediately. Thanks.
> >
> 
> "fs: convert return type int to vm_fault_t" is still under
> review/discusson and not yet merge
> into linux-next. I am not seeing it into linux-next tree.Can you
> please share the commit id ?

As Ted wrote, it is currently commit
83c0adddcc6ed128168e7b87eaed0c21eac908e4, and it came from mm tree. But the
commit ID is going to change tomorrow since linux-next is rebuilt every
day... I'm not sure how that patch got to mm tree. That's a question for
Andrew but you should have been notified about that by his mailing
machinery.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05 13:34     ` Theodore Y. Ts'o
@ 2018-09-05 18:44       ` Christoph Hellwig
  2018-09-05 19:24       ` Souptick Joarder
  1 sibling, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2018-09-05 18:44 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Souptick Joarder, Jan Kara,
	syzbot+87a05ae4accd500f5242, ak, Andrew Morton, linux-kernel,
	Linux-MM, mawilcox, mgorman, syzkaller-bugs, tim.c.chen, zwisler,
	willy

On Wed, Sep 05, 2018 at 09:34:59AM -0400, Theodore Y. Ts'o wrote:
> It's at: 83c0adddcc6ed128168e7b87eaed0c21eac908e4 in the Linux Next
> branch.
> 
> Dmitry, can you try reverting this commit and see if it makes the
> problem go away?
> 
> Souptick, can we just NACK this patch and completely drop it from all
> trees?
> 
> I think we need to be a *lot* more careful about this vm_fault_t patch
> thing.  If you can't be bothered to run xfstests, we need to introduce
> a new function which replaces block_page_mkwrite() --- and then let
> each file system try to convert over to it at their own pace, after
> they've done regression testing.

block_page_mkwrite is only called by ext4 and nilfs2 anyway, so
converting both callers over should not be a problem, as long as
it actually is done properly.

Which is my main beef with this mess of a conversation - it should
have been posted as a single series that actually does a mostly
scriped conversion after fixing up the initial harder issues, and
be properly tested.  It has been pretty much an example of how not
do things, and been dragging on forever while wasting everyones time.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05  8:55 ` Jan Kara
  2018-09-05  9:50   ` Souptick Joarder
@ 2018-09-05 19:07   ` Souptick Joarder
  2018-09-06  8:12     ` Jan Kara
  1 sibling, 1 reply; 19+ messages in thread
From: Souptick Joarder @ 2018-09-05 19:07 UTC (permalink / raw)
  To: Jan Kara
  Cc: syzbot+87a05ae4accd500f5242, ak, Andrew Morton, linux-kernel,
	Linux-MM, mawilcox, mgorman, syzkaller-bugs, tim.c.chen, zwisler

On Wed, Sep 5, 2018 at 2:25 PM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 05-09-18 00:13:02, syzbot wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
> > git tree:       linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
> > dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
> > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> >
> > Unfortunately, I don't have any reproducer for this crash yet.
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com
> >
> > INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for
> > more than 140 seconds.
> >       Not tainted 4.19.0-rc2-next-20180905+ #56
> > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > syz-fuzzer      D21704  4876   4871 0x00000000
> > Call Trace:
> >  context_switch kernel/sched/core.c:2825 [inline]
> >  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
> >  schedule+0xfb/0x450 kernel/sched/core.c:3517
> >  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
> >  wait_on_page_bit_common mm/filemap.c:1100 [inline]
> >  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
> >  lock_page include/linux/pagemap.h:483 [inline]
> >  do_page_mkwrite+0x429/0x520 mm/memory.c:2391
>
> Waiting for page lock after ->page_mkwrite callback. Which means
> ->page_mkwrite did not return VM_FAULT_LOCKED but 0. Looking into
> linux-next... indeed "fs: convert return type int to vm_fault_t" has busted
> block_page_mkwrite(). It has to return VM_FAULT_LOCKED and not 0 now.
> Souptick, can I ask you to run 'fstests' for at least common filesystems
> like ext4, xfs, btrfs when you change generic filesystem code please? That
> would catch a bug like this immediately. Thanks.

Looking into existing code block_page_mkwrite() returns 0, not VM_FAULT_LOCKED
in true path and this patch doesn't change any existing behaviour of
block_page_mkwrite()
except adding one new input parameter to return err value to caller function.

-int ext4_page_mkwrite(struct vm_fault *vmf)
+vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)

+       err = 0;
+       ret = block_page_mkwrite(vma, vmf, get_block, &err);
        if (!ret && ext4_should_journal_data(inode)) {
                if (ext4_walk_page_buffers(handle, page_buffers(page), 0,
                          PAGE_SIZE, NULL, do_journal_get_write_access)) {
                        unlock_page(page);
-                       ret = VM_FAULT_SIGBUS;

I think, this part has created problem where page_mkwrite()
end up with returning 0.

Correct me if I am wrong.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05 13:34     ` Theodore Y. Ts'o
  2018-09-05 18:44       ` Christoph Hellwig
@ 2018-09-05 19:24       ` Souptick Joarder
  2018-09-06  8:38         ` Jan Kara
  1 sibling, 1 reply; 19+ messages in thread
From: Souptick Joarder @ 2018-09-05 19:24 UTC (permalink / raw)
  To: Theodore Ts'o, Jan Kara, syzbot+87a05ae4accd500f5242, ak,
	Andrew Morton, linux-kernel, Linux-MM, mawilcox, mgorman,
	syzkaller-bugs, tim.c.chen, zwisler, Matthew Wilcox

On Wed, Sep 5, 2018 at 7:05 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Wed, Sep 05, 2018 at 03:20:16PM +0530, Souptick Joarder wrote:
> >
> > "fs: convert return type int to vm_fault_t" is still under
> > review/discusson and not yet merge
> > into linux-next. I am not seeing it into linux-next tree.Can you
> > please share the commit id ?
>
> It's at: 83c0adddcc6ed128168e7b87eaed0c21eac908e4 in the Linux Next
> branch.
>
> Dmitry, can you try reverting this commit and see if it makes the
> problem go away?
>
> Souptick, can we just NACK this patch and completely drop it from all
> trees?

Ok, I will correct it and post v3.

>
> I think we need to be a *lot* more careful about this vm_fault_t patch
> thing.  If you can't be bothered to run xfstests, we need to introduce
> a new function which replaces block_page_mkwrite() --- and then let
> each file system try to convert over to it at their own pace, after
> they've done regression testing.
>
>                                                 - Ted

Chris has his opinion,

block_page_mkwrite is only called by ext4 and nilfs2 anyway, so
converting both callers over should not be a problem, as long as
it actually is done properly.

Matthew's opinion in other mail thread -

> +vm_fault_t block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
> +                      get_block_t get_block, int *err)

I don't like returning both the errno and the vm_fault_t.  To me that's a
sign we need to rethink this interface.

I have two suggestions.  First, we could allocate a new VM_FAULT_NOSPC
bit.  Second, we could repurpose one of the existing bits, such as
VM_FAULT_RETRY for this purpose.

> -int ext4_page_mkwrite(struct vm_fault *vmf)
> +vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)

I also think perhaps we could start by _not_ converting block_page_mkwrite().
Just convert ext4_page_mkwrite(), and save converting block_page_mkwrite()
for later.

Which approach Shall I take ??

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05 19:07   ` Souptick Joarder
@ 2018-09-06  8:12     ` Jan Kara
  2018-09-06 12:02       ` Souptick Joarder
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Kara @ 2018-09-06  8:12 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Jan Kara, syzbot+87a05ae4accd500f5242, ak, Andrew Morton,
	linux-kernel, Linux-MM, mawilcox, mgorman, syzkaller-bugs,
	tim.c.chen, zwisler

On Thu 06-09-18 00:37:06, Souptick Joarder wrote:
> On Wed, Sep 5, 2018 at 2:25 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Wed 05-09-18 00:13:02, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
> > > git tree:       linux-next
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
> > > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> > >
> > > Unfortunately, I don't have any reproducer for this crash yet.
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com
> > >
> > > INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for
> > > more than 140 seconds.
> > >       Not tainted 4.19.0-rc2-next-20180905+ #56
> > > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > > syz-fuzzer      D21704  4876   4871 0x00000000
> > > Call Trace:
> > >  context_switch kernel/sched/core.c:2825 [inline]
> > >  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
> > >  schedule+0xfb/0x450 kernel/sched/core.c:3517
> > >  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
> > >  wait_on_page_bit_common mm/filemap.c:1100 [inline]
> > >  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
> > >  lock_page include/linux/pagemap.h:483 [inline]
> > >  do_page_mkwrite+0x429/0x520 mm/memory.c:2391
> >
> > Waiting for page lock after ->page_mkwrite callback. Which means
> > ->page_mkwrite did not return VM_FAULT_LOCKED but 0. Looking into
> > linux-next... indeed "fs: convert return type int to vm_fault_t" has busted
> > block_page_mkwrite(). It has to return VM_FAULT_LOCKED and not 0 now.
> > Souptick, can I ask you to run 'fstests' for at least common filesystems
> > like ext4, xfs, btrfs when you change generic filesystem code please? That
> > would catch a bug like this immediately. Thanks.
> 
> Looking into existing code block_page_mkwrite() returns 0, not VM_FAULT_LOCKED
> in true path and this patch doesn't change any existing behaviour of
> block_page_mkwrite()
> except adding one new input parameter to return err value to caller function.

Yeah, you are right and this confused me. In your version
block_page_mkwrite() returns block_page_mkwrite_return(err1) in case of
error but 0 in case of success and the caller - ext4_page_mkwrite() - then
uses block_page_mkwrite_return() again if block_page_mkwrite() returned 0.
So I agree the code path I pointed out won't result in returning 0 instead
of VM_FAULT_LOCKED but the calling convention is really very confusing.

> -int ext4_page_mkwrite(struct vm_fault *vmf)
> +vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)
> 
> +       err = 0;
> +       ret = block_page_mkwrite(vma, vmf, get_block, &err);
>         if (!ret && ext4_should_journal_data(inode)) {
>                 if (ext4_walk_page_buffers(handle, page_buffers(page), 0,
>                           PAGE_SIZE, NULL, do_journal_get_write_access)) {
>                         unlock_page(page);
> -                       ret = VM_FAULT_SIGBUS;
> 
> I think, this part has created problem where page_mkwrite()
> end up with returning 0.

So this branch is definitely wrong but I somewhat doubt it's the one we've
taken - this can happen only in case of IO error.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-05 19:24       ` Souptick Joarder
@ 2018-09-06  8:38         ` Jan Kara
  2018-09-06 12:26           ` Souptick Joarder
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Kara @ 2018-09-06  8:38 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Theodore Ts'o, Jan Kara, syzbot+87a05ae4accd500f5242, ak,
	Andrew Morton, linux-kernel, Linux-MM, mgorman, syzkaller-bugs,
	tim.c.chen, zwisler, Matthew Wilcox

On Thu 06-09-18 00:54:50, Souptick Joarder wrote:
> On Wed, Sep 5, 2018 at 7:05 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
> >
> > On Wed, Sep 05, 2018 at 03:20:16PM +0530, Souptick Joarder wrote:
> > >
> > > "fs: convert return type int to vm_fault_t" is still under
> > > review/discusson and not yet merge
> > > into linux-next. I am not seeing it into linux-next tree.Can you
> > > please share the commit id ?
> >
> > It's at: 83c0adddcc6ed128168e7b87eaed0c21eac908e4 in the Linux Next
> > branch.
> >
> > Dmitry, can you try reverting this commit and see if it makes the
> > problem go away?
> >
> > Souptick, can we just NACK this patch and completely drop it from all
> > trees?
> 
> Ok, I will correct it and post v3.
> 
> >
> > I think we need to be a *lot* more careful about this vm_fault_t patch
> > thing.  If you can't be bothered to run xfstests, we need to introduce
> > a new function which replaces block_page_mkwrite() --- and then let
> > each file system try to convert over to it at their own pace, after
> > they've done regression testing.
> >
> >                                                 - Ted
> 
> Chris has his opinion,
> 
> block_page_mkwrite is only called by ext4 and nilfs2 anyway, so
> converting both callers over should not be a problem, as long as
> it actually is done properly.
> 
> Matthew's opinion in other mail thread -
> 
> > +vm_fault_t block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
> > +                      get_block_t get_block, int *err)
> 
> I don't like returning both the errno and the vm_fault_t.  To me that's a
> sign we need to rethink this interface.
> 
> I have two suggestions.  First, we could allocate a new VM_FAULT_NOSPC
> bit.  Second, we could repurpose one of the existing bits, such as
> VM_FAULT_RETRY for this purpose.
> 
> > -int ext4_page_mkwrite(struct vm_fault *vmf)
> > +vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)
> 
> I also think perhaps we could start by _not_ converting block_page_mkwrite().
> Just convert ext4_page_mkwrite(), and save converting block_page_mkwrite()
> for later.

Yes, I'd start with converting ext4_page_mkwrite() - that should be pretty
straightforward - and we can leave block_page_mkwrite() as is for now. I
don't think allocating other VM_FAULT_ codes is going to cut it as
generally the filesystem may need to communicate different error codes back
and you don't know in advance which are interesting.

One solution for passing error codes we could use with vm_fault_t is a
scheme similar to ERR_PTR. So we can store full error code in vm_fault_t
and still have a plenty of space for the special VM_FAULT_ return codes...
With that scheme converting block_page_mkwrite() would be trivial.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-06  8:12     ` Jan Kara
@ 2018-09-06 12:02       ` Souptick Joarder
  0 siblings, 0 replies; 19+ messages in thread
From: Souptick Joarder @ 2018-09-06 12:02 UTC (permalink / raw)
  To: Jan Kara
  Cc: syzbot+87a05ae4accd500f5242, ak, Andrew Morton, linux-kernel,
	Linux-MM, mawilcox, mgorman, syzkaller-bugs, tim.c.chen, zwisler

On Thu, Sep 6, 2018 at 1:42 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 06-09-18 00:37:06, Souptick Joarder wrote:
> > On Wed, Sep 5, 2018 at 2:25 PM Jan Kara <jack@suse.cz> wrote:
> > >
> > > On Wed 05-09-18 00:13:02, syzbot wrote:
> > > > Hello,
> > > >
> > > > syzbot found the following crash on:
> > > >
> > > > HEAD commit:    387ac6229ecf Add linux-next specific files for 20180905
> > > > git tree:       linux-next
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=149c67a6400000
> > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=ad5163873ecfbc32
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=87a05ae4accd500f5242
> > > > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> > > >
> > > > Unfortunately, I don't have any reproducer for this crash yet.
> > > >
> > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > Reported-by: syzbot+87a05ae4accd500f5242@syzkaller.appspotmail.com
> > > >
> > > > INFO: task hung in do_page_mkwriteINFO: task syz-fuzzer:4876 blocked for
> > > > more than 140 seconds.
> > > >       Not tainted 4.19.0-rc2-next-20180905+ #56
> > > > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > > > syz-fuzzer      D21704  4876   4871 0x00000000
> > > > Call Trace:
> > > >  context_switch kernel/sched/core.c:2825 [inline]
> > > >  __schedule+0x87c/0x1df0 kernel/sched/core.c:3473
> > > >  schedule+0xfb/0x450 kernel/sched/core.c:3517
> > > >  io_schedule+0x1c/0x70 kernel/sched/core.c:5140
> > > >  wait_on_page_bit_common mm/filemap.c:1100 [inline]
> > > >  __lock_page+0x5b7/0x7a0 mm/filemap.c:1273
> > > >  lock_page include/linux/pagemap.h:483 [inline]
> > > >  do_page_mkwrite+0x429/0x520 mm/memory.c:2391
> > >
> > > Waiting for page lock after ->page_mkwrite callback. Which means
> > > ->page_mkwrite did not return VM_FAULT_LOCKED but 0. Looking into
> > > linux-next... indeed "fs: convert return type int to vm_fault_t" has busted
> > > block_page_mkwrite(). It has to return VM_FAULT_LOCKED and not 0 now.
> > > Souptick, can I ask you to run 'fstests' for at least common filesystems
> > > like ext4, xfs, btrfs when you change generic filesystem code please? That
> > > would catch a bug like this immediately. Thanks.
> >
> > Looking into existing code block_page_mkwrite() returns 0, not VM_FAULT_LOCKED
> > in true path and this patch doesn't change any existing behaviour of
> > block_page_mkwrite()
> > except adding one new input parameter to return err value to caller function.
>
> Yeah, you are right and this confused me. In your version
> block_page_mkwrite() returns block_page_mkwrite_return(err1) in case of
> error but 0 in case of success and the caller - ext4_page_mkwrite() - then
> uses block_page_mkwrite_return() again if block_page_mkwrite() returned 0.
> So I agree the code path I pointed out won't result in returning 0 instead
> of VM_FAULT_LOCKED but the calling convention is really very confusing.
>
> > -int ext4_page_mkwrite(struct vm_fault *vmf)
> > +vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)
> >
> > +       err = 0;
> > +       ret = block_page_mkwrite(vma, vmf, get_block, &err);
> >         if (!ret && ext4_should_journal_data(inode)) {
> >                 if (ext4_walk_page_buffers(handle, page_buffers(page), 0,
> >                           PAGE_SIZE, NULL, do_journal_get_write_access)) {
> >                         unlock_page(page);
> > -                       ret = VM_FAULT_SIGBUS;
> >
> > I think, this part has created problem where page_mkwrite()
> > end up with returning 0.
>
> So this branch is definitely wrong but I somewhat doubt it's the one we've
> taken - this can happen only in case of IO error.

Looking into the patch, this is only part of code where page_mkwrite()
end up with returning 0.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-06  8:38         ` Jan Kara
@ 2018-09-06 12:26           ` Souptick Joarder
  2018-09-06 13:05             ` Matthew Wilcox
  2018-09-06 13:12             ` Theodore Y. Ts'o
  0 siblings, 2 replies; 19+ messages in thread
From: Souptick Joarder @ 2018-09-06 12:26 UTC (permalink / raw)
  To: Jan Kara
  Cc: Theodore Ts'o, syzbot+87a05ae4accd500f5242, ak,
	Andrew Morton, linux-kernel, Linux-MM, mgorman, syzkaller-bugs,
	tim.c.chen, zwisler, Matthew Wilcox

On Thu, Sep 6, 2018 at 2:08 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 06-09-18 00:54:50, Souptick Joarder wrote:
> > On Wed, Sep 5, 2018 at 7:05 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
> > >
> > > On Wed, Sep 05, 2018 at 03:20:16PM +0530, Souptick Joarder wrote:
> > > >
> > > > "fs: convert return type int to vm_fault_t" is still under
> > > > review/discusson and not yet merge
> > > > into linux-next. I am not seeing it into linux-next tree.Can you
> > > > please share the commit id ?
> > >
> > > It's at: 83c0adddcc6ed128168e7b87eaed0c21eac908e4 in the Linux Next
> > > branch.
> > >
> > > Dmitry, can you try reverting this commit and see if it makes the
> > > problem go away?
> > >
> > > Souptick, can we just NACK this patch and completely drop it from all
> > > trees?
> >
> > Ok, I will correct it and post v3.
> >
> > >
> > > I think we need to be a *lot* more careful about this vm_fault_t patch
> > > thing.  If you can't be bothered to run xfstests, we need to introduce
> > > a new function which replaces block_page_mkwrite() --- and then let
> > > each file system try to convert over to it at their own pace, after
> > > they've done regression testing.
> > >
> > >                                                 - Ted
> >
> > Chris has his opinion,
> >
> > block_page_mkwrite is only called by ext4 and nilfs2 anyway, so
> > converting both callers over should not be a problem, as long as
> > it actually is done properly.
> >
> > Matthew's opinion in other mail thread -
> >
> > > +vm_fault_t block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
> > > +                      get_block_t get_block, int *err)
> >
> > I don't like returning both the errno and the vm_fault_t.  To me that's a
> > sign we need to rethink this interface.
> >
> > I have two suggestions.  First, we could allocate a new VM_FAULT_NOSPC
> > bit.  Second, we could repurpose one of the existing bits, such as
> > VM_FAULT_RETRY for this purpose.
> >
> > > -int ext4_page_mkwrite(struct vm_fault *vmf)
> > > +vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)
> >
> > I also think perhaps we could start by _not_ converting block_page_mkwrite().
> > Just convert ext4_page_mkwrite(), and save converting block_page_mkwrite()
> > for later.
>

> Yes, I'd start with converting ext4_page_mkwrite() - that should be pretty
> straightforward - and we can leave block_page_mkwrite() as is for now. I
> don't think allocating other VM_FAULT_ codes is going to cut it as
> generally the filesystem may need to communicate different error codes back
> and you don't know in advance which are interesting.

Then I need to take care of ext4_page_mkwrite() and ext4_filemap_fault()
to migrate to use vm_fault_t return type. Everything else can be removed
from this patch and it will go as a separate patch.

As block_page_mkwrite() is getting called from 2 places in ext4 and nilfs and
both places fault handler code convert errno to VM_FAULT_CODE using
block_page_mkwrite_return(), is it required to migrate block_page_mkwrite()
to use vm_fault_t return type and further complicate the API or better to
leave this API in current state ??

>
> One solution for passing error codes we could use with vm_fault_t is a
> scheme similar to ERR_PTR. So we can store full error code in vm_fault_t
> and still have a plenty of space for the special VM_FAULT_ return codes...
> With that scheme converting block_page_mkwrite() would be trivial.
>
I didn't get this part. Any reference code will be helpful ?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-06 12:26           ` Souptick Joarder
@ 2018-09-06 13:05             ` Matthew Wilcox
  2018-09-06 13:12             ` Theodore Y. Ts'o
  1 sibling, 0 replies; 19+ messages in thread
From: Matthew Wilcox @ 2018-09-06 13:05 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Jan Kara, Theodore Ts'o, syzbot+87a05ae4accd500f5242, ak,
	Andrew Morton, linux-kernel, Linux-MM, mgorman, syzkaller-bugs,
	tim.c.chen, zwisler

On Thu, Sep 06, 2018 at 05:56:31PM +0530, Souptick Joarder wrote:
> On Thu, Sep 6, 2018 at 2:08 PM Jan Kara <jack@suse.cz> wrote:
> > Yes, I'd start with converting ext4_page_mkwrite() - that should be pretty
> > straightforward - and we can leave block_page_mkwrite() as is for now. I
> > don't think allocating other VM_FAULT_ codes is going to cut it as
> > generally the filesystem may need to communicate different error codes back
> > and you don't know in advance which are interesting.
> 
> Then I need to take care of ext4_page_mkwrite() and ext4_filemap_fault()
> to migrate to use vm_fault_t return type. Everything else can be removed
> from this patch and it will go as a separate patch.
> 
> As block_page_mkwrite() is getting called from 2 places in ext4 and nilfs and
> both places fault handler code convert errno to VM_FAULT_CODE using
> block_page_mkwrite_return(), is it required to migrate block_page_mkwrite()
> to use vm_fault_t return type and further complicate the API or better to
> leave this API in current state ??

Leave block_page_mkwrite() alone.  Somebody who understands it better
than you do can take care of converting it, if that's even the right
thing to do.  Let's get the typedef conversion _finished_ so we get the
benefit of typechecking for driver writers.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-06 12:26           ` Souptick Joarder
  2018-09-06 13:05             ` Matthew Wilcox
@ 2018-09-06 13:12             ` Theodore Y. Ts'o
  2018-09-06 14:07               ` Theodore Y. Ts'o
  2018-09-06 16:00               ` Matthew Wilcox
  1 sibling, 2 replies; 19+ messages in thread
From: Theodore Y. Ts'o @ 2018-09-06 13:12 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Jan Kara, syzbot+87a05ae4accd500f5242, ak, Andrew Morton,
	linux-kernel, Linux-MM, mgorman, syzkaller-bugs, tim.c.chen,
	zwisler, Matthew Wilcox

On Thu, Sep 06, 2018 at 05:56:31PM +0530, Souptick Joarder wrote:
> > Yes, I'd start with converting ext4_page_mkwrite() - that should be pretty
> > straightforward - and we can leave block_page_mkwrite() as is for now. I
> > don't think allocating other VM_FAULT_ codes is going to cut it as
> > generally the filesystem may need to communicate different error codes back
> > and you don't know in advance which are interesting.

Changing the return values of ext4_page_mkwrite() and
ext4_filemap_fault() is definitely safe.  If you want to start
changing the type of "ret" to vm_fault_t and introduce a new variable
"err", now you have to be super careful not to screw things up.  (I
believe one of the earlier patches didn't get that right.)

> As block_page_mkwrite() is getting called from 2 places in ext4 and nilfs and
> both places fault handler code convert errno to VM_FAULT_CODE using
> block_page_mkwrite_return(), is it required to migrate block_page_mkwrite()
> to use vm_fault_t return type and further complicate the API or better to
> leave this API in current state ??

So I don't see the point of changing return value block_page_mkwrite()
(although to be honest I haven't see the value of the vm_fault_t
change at all in the first place, at least not compared to the pain it
has caused) but no, I don't think it's worth it.

The API for block_page_mkwrite() can simply be defined as "0 on
success, < 0 on error".  You can add documentation that it's up to
caller of block_page_mkwrite() to call block_page_mkwrite_return() to
translate the error to a vm_fault_t.

> > One solution for passing error codes we could use with vm_fault_t is a
> > scheme similar to ERR_PTR. So we can store full error code in vm_fault_t
> > and still have a plenty of space for the special VM_FAULT_ return codes...
> > With that scheme converting block_page_mkwrite() would be trivial.
> >
> I didn't get this part. Any reference code will be helpful ?

So what we do for functions that need to either return an error or a
pointer is to call encode the error as a "pointer" by using ERR_PTR(),
and the caller can determine whether or not it is a valid pointer or
an error code by using IS_ERR_VALUE() and turning it back into an
error by using PTR_ERR().   See include/linux/err.h.

Similarly, all valid vm_fault_t's composed of VM_FAULT_xxx are
positive integers, and all errors are passed using the kernel's
convention of using a negative error code.  So going through lots of
machinations to return both an error code and a vm_fault_t *really*
wasn't necessary.

The issue, as near as I can understand things, for why we're going
through all of this churn, was there was a concern that in the mm
code, that all of the places which received a vm_fault_t would
sometimes see a negative error code.  The proposal here is to just
*accept* that this will happen, and just simply have them *check* to
see if it's a negative error code, and convert it to the appropriate
vm_fault_t in that case.  It puts the onus of the change on the mm
layer, where as the "blast radius" of the vm_fault_t "cleanup" is
spread out across a large number of subsystems.

Which I wouldn't mind, if it wasn't causing pain.  But it *is* causing
pain.

And it's common kernel convention to overload an error and a pointer
using the exact same trick.  We do it *all* over the place, and quite
frankly, it's less error prone than changing functions to return a
pointer and an error.  No one has said, "let's do to the ERR_PTR
convention what we've done to the vm_fault_t -- it's too confusing
that a pointer might be an error, since people might forget to check
for it."  If they did that, it would be NACK'ed right, left and
center.  But yet it's a good idea for vm_fault_t?

	     	      	     	      - Ted

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-06 13:12             ` Theodore Y. Ts'o
@ 2018-09-06 14:07               ` Theodore Y. Ts'o
  2018-09-06 16:00               ` Matthew Wilcox
  1 sibling, 0 replies; 19+ messages in thread
From: Theodore Y. Ts'o @ 2018-09-06 14:07 UTC (permalink / raw)
  To: Souptick Joarder, Jan Kara, syzbot+87a05ae4accd500f5242, ak,
	Andrew Morton, linux-kernel, Linux-MM, mgorman, syzkaller-bugs,
	tim.c.chen, zwisler, Matthew Wilcox

P.S.  This is the second time the vm_fualt_t change has broken things.
The first time, when it went through the ext4 tree, I NACK'ed it after
a 60 seconds smoke test showed it was broken.  This time it went
through the mm tree...

In the future, even for "trivial" changes, could you *please* run the
kvm-xfstests[1] or gce-xfstests[2][3]?

[1] https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md
[2] https://github.com/tytso/xfstests-bld/blob/master/Documentation/gce-xfstests.md
[3] https:/thunk.org/gce-xfstests

Or if you're too lazy to run the smoke tests, please send it through
the ext4 tree so *I* can run the smoke tests.

						- Ted

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: linux-next test error
  2018-09-06 13:12             ` Theodore Y. Ts'o
  2018-09-06 14:07               ` Theodore Y. Ts'o
@ 2018-09-06 16:00               ` Matthew Wilcox
  1 sibling, 0 replies; 19+ messages in thread
From: Matthew Wilcox @ 2018-09-06 16:00 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Souptick Joarder, Jan Kara,
	syzbot+87a05ae4accd500f5242, ak, Andrew Morton, linux-kernel,
	Linux-MM, mgorman, syzkaller-bugs, tim.c.chen, zwisler

On Thu, Sep 06, 2018 at 09:12:12AM -0400, Theodore Y. Ts'o wrote:
> So I don't see the point of changing return value block_page_mkwrite()
> (although to be honest I haven't see the value of the vm_fault_t
> change at all in the first place, at least not compared to the pain it
> has caused) but no, I don't think it's worth it.

You have a sampling bias though; you've only seen the filesystem patches.
Filesystem fault handlers are generally more complex and written by
people who have more Linux expertise.  For the device drivers, it's
been far more useful; bugs have been fixed and a lot of cargo-culted
code has been deleted.

> So what we do for functions that need to either return an error or a
> pointer is to call encode the error as a "pointer" by using ERR_PTR(),
> and the caller can determine whether or not it is a valid pointer or
> an error code by using IS_ERR_VALUE() and turning it back into an
> error by using PTR_ERR().   See include/linux/err.h.

That's _usually_ the convention when a function might return a pointer
or an error.  Sometimes we return NULL to mean "an error happened".
Sometimes that NULL means -ENOMEM.  Sometimes we return ZERO_SIZE_PTR
instead of -EINVAL.  Sometimes we return a POISON value.  It's all pretty
ad-hoc, which wouldn't be as bad if it were better documented.

> Similarly, all valid vm_fault_t's composed of VM_FAULT_xxx are
> positive integers, and all errors are passed using the kernel's
> convention of using a negative error code.  So going through lots of
> machinations to return both an error code and a vm_fault_t *really*
> wasn't necessary.

Not necessary from the point of view that there are enough bits to be able
to distinguish the two, I agree.  But from the mm point of view, it rather
does matter that you can distinguish between SIGBUS, SIGSEGV, HWPOISON
and OOM (although -ENOMEM and VM_FAULT_OOM do have the same meaning).

> The issue, as near as I can understand things, for why we're going
> through all of this churn, was there was a concern that in the mm
> code, that all of the places which received a vm_fault_t would
> sometimes see a negative error code.  The proposal here is to just
> *accept* that this will happen, and just simply have them *check* to
> see if it's a negative error code, and convert it to the appropriate
> vm_fault_t in that case.  It puts the onus of the change on the mm
> layer, where as the "blast radius" of the vm_fault_t "cleanup" is
> spread out across a large number of subsystems.
> 
> Which I wouldn't mind, if it wasn't causing pain.  But it *is* causing
> pain.

As I said earlier, your sample bias shows only pain, but there are
genuine improvements in the patches you haven't seen and don't care about.

> And it's common kernel convention to overload an error and a pointer
> using the exact same trick.  We do it *all* over the place, and quite
> frankly, it's less error prone than changing functions to return a
> pointer and an error.  No one has said, "let's do to the ERR_PTR
> convention what we've done to the vm_fault_t -- it's too confusing
> that a pointer might be an error, since people might forget to check
> for it."  If they did that, it would be NACK'ed right, left and
> center.  But yet it's a good idea for vm_fault_t?

I actually think it would be a good idea to mark functions which return
either-an-errno-or-a-pointer as returning an errptr_t.  The downside is
that we'd lose the type information (we'd only know that it's a void *
or an errno, not that it's a struct ext4_foo * or an errno).  Just like
we gradually introduced 'bool' instead of 'int' for functions which only
returned true/false.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-09-06 16:01 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-05  7:13 linux-next test error syzbot
2018-09-05  7:17 ` Dmitry Vyukov
2018-09-05  8:55 ` Jan Kara
2018-09-05  9:50   ` Souptick Joarder
2018-09-05  9:53     ` Dmitry Vyukov
2018-09-05  9:57       ` Dmitry Vyukov
2018-09-05 13:34     ` Theodore Y. Ts'o
2018-09-05 18:44       ` Christoph Hellwig
2018-09-05 19:24       ` Souptick Joarder
2018-09-06  8:38         ` Jan Kara
2018-09-06 12:26           ` Souptick Joarder
2018-09-06 13:05             ` Matthew Wilcox
2018-09-06 13:12             ` Theodore Y. Ts'o
2018-09-06 14:07               ` Theodore Y. Ts'o
2018-09-06 16:00               ` Matthew Wilcox
2018-09-05 13:55     ` Jan Kara
2018-09-05 19:07   ` Souptick Joarder
2018-09-06  8:12     ` Jan Kara
2018-09-06 12:02       ` Souptick Joarder

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.