Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

blkdev.h: revalidate_disk removed #11967

Closed
65a opened this issue Apr 29, 2021 · 12 comments · Fixed by #11977
Closed

blkdev.h: revalidate_disk removed #11967

65a opened this issue Apr 29, 2021 · 12 comments · Fixed by #11977
Labels
Type: Building Indicates an issue related to building binaries

Comments

@65a
Copy link

65a commented Apr 29, 2021

torvalds/linux@0f00b82#diff-79b436371fdb3ddf0e7ad9bd4c9afe05160f7953438e650a77519b882904c56b

DKMS is failing on latest kernel sources.

@behlendorf behlendorf added the Type: Building Indicates an issue related to building binaries label Apr 29, 2021
@behlendorf
Copy link
Contributor

@ckane are you interested in taking a look at this?

@ckane
Copy link
Contributor

ckane commented Apr 30, 2021

thx I can check it out - is this an upcoming 5.13 regression?

@ckane
Copy link
Contributor

ckane commented Apr 30, 2021

thx I can check it out - is this an upcoming 5.13 regression?

nvm I see the commit history above - I'll see if I can get this fixed tonight

@ckane
Copy link
Contributor

ckane commented Apr 30, 2021

So the existing call in our code says the following:

     /*
      * Force revalidation, to mimic the old behavior of
      * check_disk_change()
      */

I'm taking this, plus the Linux kernel commit earlier, to potentially indicate it is safe to remove this in cases where the struct block_device_operations no longer contains a handler function for revaidate_disk. It looks like what happens in the code today is that this ultimately calls zvol_revalidate_disk - and if so an alternative might be to call that directly, but I don't see anywhere else in this code where we call into zvol. I'll test my hypothesis that this works by just removing the call, the handler method and its initialization, and see how that works.

@ckane
Copy link
Contributor

ckane commented Apr 30, 2021

Ok PR now linked above, passes checkstyle - so I'll let it run the other automated before I test it on my system tomorrow.

@65a
Copy link
Author

65a commented May 1, 2021

I have the patched version running successfully for several hours across three nodes with no issues, but I am not doing anything fancy with disks (e.g. removal, reboots, hotswap, etc) or zvols (any changes).

@65a
Copy link
Author

65a commented May 1, 2021

I eventually received the following WARNING in the zfs module
[54359.068536] ------------[ cut here ]------------ [54359.068540] Attempted to advance past end of bvec iter [54359.068549] WARNING: CPU: 5 PID: 805726 at include/linux/bvec.h:105 iov_iter_advance+0x5f8/0x610
... SNIP ...
[54359.068630] RIP: 0010:iov_iter_advance+0x5f8/0x610 [54359.068632] Code: a7 fc ff ff 4c 89 df 45 31 c0 e9 55 fd ff ff 49 89 f0 e9 83 fd ff ff 48 c7 c7 48 a6 64 a9 c6 05 b8 c3 05 01 01 e8 ec eb 58 00 <0f> 0b 4c 8b 4b 18 eb b6 45 31 e4 e9 ca fe ff ff e8 63 d5 5d 00 0f [54359.068635] RSP: 0018:ffffb06eab2d7b08 EFLAGS: 00010286 [54359.068637] RAX: 0000000000000000 RBX: ffffb06eab2d7cc8 RCX: ffff94b11fb58528 [54359.068639] RDX: 00000000ffffffd8 RSI: 0000000000000027 RDI: ffff94b11fb58520 [54359.068640] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffb06eab2d7940 [54359.068641] R10: ffffb06eab2d7938 R11: ffffffffa98cae68 R12: ffff94aa57c70540 [54359.068643] R13: ffff94a9d2d04d80 R14: ffffb06eab2d7b18 R15: ffffb06eab2d7cc8 [54359.068644] FS: 00000000030b9b10(0000) GS:ffff94b11fb40000(0000) knlGS:0000000000000000 [54359.068646] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [54359.068648] CR2: 000000000049eec0 CR3: 00000001cf0ca006 CR4: 00000000003706e0 [54359.068649] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [54359.068650] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [54359.068652] Call Trace: [54359.068656] zpl_iter_write+0x186/0x1b0 [zfs] [54359.068706] do_iter_readv_writev+0x12f/0x190 [54359.068710] do_iter_write+0x77/0x1b0 [54359.068713] iter_file_splice_write+0x2bd/0x440 [54359.068717] direct_splice_actor+0x27/0x40 [54359.068720] splice_direct_to_actor+0xf8/0x220 [54359.068723] ? do_splice_direct+0xd0/0xd0 [54359.068725] do_splice_direct+0x86/0xd0 [54359.068728] vfs_copy_file_range+0x1d7/0x4e0 [54359.068731] __do_sys_copy_file_range+0xd3/0x210 [54359.068734] do_syscall_64+0x6b/0x80 [54359.068738] ? do_syscall_64+0x11/0x80 [54359.068740] entry_SYSCALL_64_after_hwframe+0x44/0xae [54359.068744] RIP: 0033:0x487a6a [54359.068746] Code: e8 3b f8 fd ff 48 8b 7c 24 10 48 8b 74 24 18 48 8b 54 24 20 4c 8b 54 24 28 4c 8b 44 24 30 4c 8b 4c 24 38 48 8b 44 24 08 0f 05 <48> 3d 01 f0 ff ff 76 20 48 c7 44 24 40 ff ff ff ff 48 c7 44 24 48 [54359.068748] RSP: 002b:000000c0008ff818 EFLAGS: 00000212 ORIG_RAX: 0000000000000146 [54359.068750] RAX: ffffffffffffffda RBX: 000000c00007d000 RCX: 0000000000487a6a [54359.068752] RDX: 0000000000000007 RSI: 0000000000000000 RDI: 0000000000000003 [54359.068753] RBP: 000000c0008ff888 R08: 0000000040000000 R09: 0000000000000000 [54359.068754] R10: 0000000000000000 R11: 0000000000000212 R12: 0000000000000008 [54359.068755] R13: 0000000000000000 R14: 00000000022ab4a0 R15: 0000000000000000 [54359.068757] ---[ end trace 24bb9831f0c0b56f ]---

@ckane
Copy link
Contributor

ckane commented May 2, 2021

Thanks - I am also seeing that too, but I was already seeing it in my dmesg prior to this change too. Thus far it doesn't seem to have caused any failure for me (though I do keep regular backups just in case).

@behlendorf you able to confirm? It looks like I see it right about the time that the ZFS mounts occur on boot, but I am fairly certain it is not related to the removal of the revalidate_disk() call.

 Attempted to advance past end of bvec iter
 WARNING: CPU: 0 PID: 165869 at include/linux/bvec.h:105 iov_iter_advance+0x39a/0x3b0
 Modules linked in: 8021q garp mrp veth xt_nat xt_tcpudp xt_conntrack xt_MASQUERADE nf_conntrack_netlink nfnetlink xfrm_user xfrm_algo xt_addrtype br_netfilter bridge st>
  videobuf2_v4l2 snd_hda_core videobuf2_common cfg80211 r8169 snd_hwdep videodev cec irqbypass snd_pcm tpm_crb crct10dif_pclmul realtek crc32_pclmul rc_core crc32c_intel>
 CPU: 0 PID: 165869 Comm: meson Tainted: P           OE     5.12.0-rc7-ckane-00014-gdba40c50f14f #3
 Hardware name: Dell Inc. G5 5505/06WDJ9, BIOS 1.4.4 09/30/2020
 RIP: 0010:iov_iter_advance+0x39a/0x3b0
 Code: 8a fe ff ff 4d 89 e1 45 31 d2 e9 42 ff ff ff 49 89 f2 e9 70 ff ff ff 48 c7 c7 f8 d0 ba bc c6 05 d4 ec 2e 01 01 e8 85 ac 56 00 <0f> 0b 4c 8b 4b 18 eb b6 66 66 2e 0>
 RSP: 0018:ffffa5d0e509fbd8 EFLAGS: 00010286
 RAX: 0000000000000000 RBX: ffffa5d0e509fd40 RCX: 0000000000000000
 RDX: 0000000000000001 RSI: ffffffffbcb7f2b7 RDI: 00000000ffffffff
 RBP: 0000000000000000 R08: 0000000000000000 R09: ffffa5d0e509fa10
 R10: ffffa5d0e509fa08 R11: ffffffffbcecc408 R12: ffff9c86f2947ec0
 R13: ffff9c8d6e361f40 R14: ffffa5d0e509fc98 R15: ffffa5d0e509fd40
 FS:  00007f4496404740(0000) GS:ffff9c954f600000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000557a3f56a6f8 CR3: 000000079ecc0000 CR4: 0000000000350ef0
 Call Trace:
  zpl_iter_write+0x184/0x1a0 [zfs]
  do_iter_readv_writev+0x130/0x190
  do_iter_write+0x7c/0x1b0
  iter_file_splice_write+0x2ba/0x430
  direct_splice_actor+0x2c/0x40
  splice_direct_to_actor+0xf6/0x220
  ? do_splice_direct+0xd0/0xd0
  do_splice_direct+0x8b/0xd0
  do_sendfile+0x322/0x4d0
  __x64_sys_sendfile64+0x63/0xc0
  do_syscall_64+0x33/0x40
  entry_SYSCALL_64_after_hwframe+0x44/0xae
 RIP: 0033:0x7f449666f98e
 Code: c3 0f 1f 00 4c 89 d2 4c 89 c6 e9 cd fd ff ff 0f 1f 44 00 00 31 c0 c3 0f 1f 44 00 00 f3 0f 1e fa 49 89 ca b8 28 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0>
 RSP: 002b:00007ffec860a878 EFLAGS: 00000246 ORIG_RAX: 0000000000000028
 RAX: ffffffffffffffda RBX: 0000000000000006 RCX: 00007f449666f98e
 RDX: 00007ffec860a880 RSI: 0000000000000004 RDI: 0000000000000006
 RBP: 0000557a3f524ff8 R08: 0000000000000000 R09: 00007f44960959d0
 R10: 0000000000800000 R11: 0000000000000246 R12: 0000000000000004
 R13: 00007ffec860a880 R14: 0000000000800000 R15: 0000557a3f14ba10

@PiMaker
Copy link

PiMaker commented May 5, 2021

Just reporting, I've seen the zpl_iter_write message for the past few boots as well. It happens either not at all, or exactly once per boot (about 50/50 for me). Latest master as built-in for 5.12 kernel.

Here's the log if it helps:

[  782.932871] ------------[ cut here ]------------
[  782.932876] Attempted to advance past end of bvec iter
[  782.932883] WARNING: CPU: 22 PID: 7711 at include/linux/bvec.h:105 iov_iter_advance+0x39c/0x3b0
[  782.932893] Modules linked in: iptable_nat xt_MASQUERADE nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c crc32c_generic bridge 8021q garp stp mrp llc rfkill nct6775 hwmon_vid intel_rapl_msr intel_rapl_common kvm_amd mxm_wmi input_leds mousedev kvm joydev snd_usb_audio snd_hda_codec_hdmi snd_usbmidi_lib snd_rawmidi snd_hda_intel snd_seq_device rapl snd_intel_dspcfg snd_hda_codec snd_hwdep pcspkr snd_hda_core snd_pcm snd_timer snd atlantic soundcore i2c_nvidia_gpu i2c_piix4 wmi evdev mac_hid zenpower(OE) uinput tun i2c_dev msr videodev mc crypto_user fuse ip_tables x_tables dm_crypt dm_mod hid_magicmouse(OE) crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel crypto_simd cryptd ahci libahci libata scsi_mod vfat fat amdgpu drm_ttm_helper ttm gpu_sched i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec drm agpgart vfio_pci irqbypass vfio_virqfd vfio_iommu_type1 vfio
[  782.932975] CPU: 22 PID: 7711 Comm: DOM Worker Tainted: P           OE     5.12.0-mainline #1
[  782.932979] Hardware name: ASUS System Product Name/PRIME TRX40-PRO, BIOS 1402 01/15/2021
[  782.932981] RIP: 0010:iov_iter_advance+0x39c/0x3b0
[  782.932985] Code: 8a fe ff ff 4d 89 e1 45 31 d2 e9 42 ff ff ff 49 89 f2 e9 70 ff ff ff 48 c7 c7 d0 1a e0 9b c6 05 1a 70 2d 01 01 e8 7c 7c 51 00 <0f> 0b 4c 8b 4b 18 eb b6 66 66 2e 0f 1f 84 00 00 00 00 00 90 41 57
[  782.932988] RSP: 0018:ffffb3a695437c40 EFLAGS: 00010286
[  782.932991] RAX: 0000000000000000 RBX: ffffb3a695437da8 RCX: 0000000000000000
[  782.932993] RDX: 0000000000000001 RSI: ffffffff9bdb01b7 RDI: 00000000ffffffff
[  782.932995] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffb3a695437a78
[  782.932996] R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000
[  782.932998] R13: ffff8f6035b238c0 R14: ffffb3a695437da8 R15: ffff8f60b01e2200
[  782.933000] FS:  00007972de236640(0000) GS:ffff8f6ead580000(0000) knlGS:0000000000000000
[  782.933002] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  782.933004] CR2: 00003e0c9633f4a8 CR3: 000000021050a000 CR4: 0000000000350ee0
[  782.933006] Call Trace:
[  782.933011]  zpl_iter_write+0x181/0x1a0
[  782.933018]  do_iter_readv_writev+0x130/0x190
[  782.933024]  do_iter_write+0x7c/0x1b0
[  782.933026]  iter_file_splice_write+0x2b9/0x430
[  782.933032]  do_splice+0x345/0x730
[  782.933036]  __do_splice+0xe7/0x160
[  782.933040]  __x64_sys_splice+0x92/0x110
[  782.933044]  do_syscall_64+0x33/0x40
[  782.933049]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  782.933054] RIP: 0033:0x7972f72505c3
[  782.933056] Code: e8 32 5c f8 ff 44 8b 4c 24 2c 4c 8b 44 24 20 89 c5 4c 8b 54 24 18 8b 54 24 28 b8 13 01 00 00 48 8b 74 24 10 8b 7c 24 08 0f 05 <48> 3d 00 f0 ff ff 77 35 89 ef 48 89 44 24 08 e8 79 5c f8 ff 48 8b
[  782.933059] RSP: 002b:00007972de231ac0 EFLAGS: 00000293 ORIG_RAX: 0000000000000113
[  782.933061] RAX: ffffffffffffffda RBX: 00007972de231a40 RCX: 00007972f72505c3
[  782.933063] RDX: 000000000000009a RSI: 0000000000000000 RDI: 00000000000000a1
[  782.933064] RBP: 0000000000000000 R08: 0000000000010000 R09: 0000000000000000
[  782.933066] R10: 0000000000000000 R11: 0000000000000293 R12: 00007972dd7b3ec0
[  782.933067] R13: 0000000000000001 R14: 00007972e6b33480 R15: fffffffffffffffc
[  782.933071] ---[ end trace 1ee94a020b695a89 ]---

@ckane
Copy link
Contributor

ckane commented May 5, 2021

thanks @PiMaker , @65a - I think this is enough to conclude that this bug was introduced before the proposed change in this issue and related PR. I will open a new GH issue later tonight on this problem, and if I have time in the next week or so I will see if I can bisect to figure out what might have introduced the error - which could be difficult as it could be ZFS or Linux 5.12.

@behlendorf I've been using this change regularly for a few days on recent builds from Linus's tree and it seems to be stable for me, so I think it is ready for merge. The above crash dumps seem to have been introduced by a completely separate recent change.

@65a
Copy link
Author

65a commented May 11, 2021

Running without issue here for 10 days continuously, but haven't tried it on any data I can't replace. Reboot, encryption, and zfs container storage (with snapshots) seem to all be working, but not really stress testing anything.

@behlendorf
Copy link
Contributor

@ckane thanks for sorting this out. The proposed change looks good to me, and I agree it wouldn't be the cause of the warnings which have been reported here.

behlendorf pushed a commit that referenced this issue May 12, 2021
Linux kernel commit 0f00b82e5413571ed225ddbccad6882d7ea60bc7 removes the
revalidate_disk() handler from struct block_device_operations. This
caused a regression, and this commit eliminates the call to it and the
assignment in the block_device_operations static handler assignment
code, when configure identifies that the kernel doesn't support that
API handler.

Reviewed-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Coleman Kane <ckane@colemankane.org>
Closes #11967 
Closes #11977
behlendorf pushed a commit to behlendorf/zfs that referenced this issue May 28, 2021
Linux kernel commit 0f00b82e5413571ed225ddbccad6882d7ea60bc7 removes the
revalidate_disk() handler from struct block_device_operations. This
caused a regression, and this commit eliminates the call to it and the
assignment in the block_device_operations static handler assignment
code, when configure identifies that the kernel doesn't support that
API handler.

Reviewed-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Coleman Kane <ckane@colemankane.org>
Closes openzfs#11967 
Closes openzfs#11977
sempervictus pushed a commit to sempervictus/zfs that referenced this issue May 31, 2021
Linux kernel commit 0f00b82e5413571ed225ddbccad6882d7ea60bc7 removes the
revalidate_disk() handler from struct block_device_operations. This
caused a regression, and this commit eliminates the call to it and the
assignment in the block_device_operations static handler assignment
code, when configure identifies that the kernel doesn't support that
API handler.

Reviewed-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Coleman Kane <ckane@colemankane.org>
Closes openzfs#11967 
Closes openzfs#11977
tonyhutter pushed a commit that referenced this issue Jun 2, 2021
Linux kernel commit 0f00b82e5413571ed225ddbccad6882d7ea60bc7 removes the
revalidate_disk() handler from struct block_device_operations. This
caused a regression, and this commit eliminates the call to it and the
assignment in the block_device_operations static handler assignment
code, when configure identifies that the kernel doesn't support that
API handler.

Reviewed-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Coleman Kane <ckane@colemankane.org>
Closes #11967
Closes #11977
(cherry picked from commit 48c7b0e)
Signed-off-by: Jonathon Fernyhough <jonathon@m2x.dev>

Co-authored-by: Coleman Kane <ckane@colemankane.org>
tonyhutter pushed a commit to tonyhutter/zfs that referenced this issue Jun 2, 2021
Linux kernel commit 0f00b82e5413571ed225ddbccad6882d7ea60bc7 removes the
revalidate_disk() handler from struct block_device_operations. This
caused a regression, and this commit eliminates the call to it and the
assignment in the block_device_operations static handler assignment
code, when configure identifies that the kernel doesn't support that
API handler.

Reviewed-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Coleman Kane <ckane@colemankane.org>
Closes openzfs#11967
Closes openzfs#11977
(cherry picked from commit 48c7b0e)
Signed-off-by: Jonathon Fernyhough <jonathon@m2x.dev>

Co-authored-by: Coleman Kane <ckane@colemankane.org>
tonyhutter pushed a commit that referenced this issue Jun 23, 2021
Linux kernel commit 0f00b82e5413571ed225ddbccad6882d7ea60bc7 removes the
revalidate_disk() handler from struct block_device_operations. This
caused a regression, and this commit eliminates the call to it and the
assignment in the block_device_operations static handler assignment
code, when configure identifies that the kernel doesn't support that
API handler.

Reviewed-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Coleman Kane <ckane@colemankane.org>
Closes #11967
Closes #11977
(cherry picked from commit 48c7b0e)
Signed-off-by: Jonathon Fernyhough <jonathon@m2x.dev>

Co-authored-by: Coleman Kane <ckane@colemankane.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Building Indicates an issue related to building binaries
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants