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

Linux 4.12 compat: fix super_setup_bdi_name() call #6148

Merged
merged 1 commit into from
May 25, 2017

Conversation

loli10K
Copy link
Contributor

@loli10K loli10K commented May 20, 2017

Description

Add a format parameter to super_setup_bdi_name() so we don't create duplicate names in /sys/devices/virtual/bdi which prevents more than one ZFS filesystem from being mounted at a time:

root@debian-8-zfs:~# uname -a
Linux debian-8-zfs 4.12.0-rc1 #1 SMP Sat May 20 12:52:46 CEST 2017 x86_64 GNU/Linux
root@debian-8-zfs:~# ls -l /sys/devices/virtual/bdi/ | grep zfs
drwxr-xr-x 3 root root 0 May 20 13:57 zfs
root@debian-8-zfs:~# zfs create $POOLNAME/fs
filesystem 'testpool/fs' can not be mounted due to error 17
cannot mount 'testpool/fs': Invalid argument
filesystem successfully created, but not mounted
root@debian-8-zfs:~# ls -l /sys/devices/virtual/bdi/ | grep zfs
drwxr-xr-x 3 root root 0 May 20 13:57 zfs
root@debian-8-zfs:~# 
root@debian-8-zfs:~# dmesg
[  150.155657] sysfs: cannot create duplicate filename '/devices/virtual/bdi/zfs'
[  150.155669] ------------[ cut here ]------------
[  150.155673] WARNING: CPU: 1 PID: 2927 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x58/0x70
[  150.155674] Modules linked in: zfs(POE) joydev(E) acpi_cpufreq(E) sg(E) evdev(E) pvpanic(E) tpm_tis(E) virtio_balloon(E) virtio_console(E) serio_raw(E) i2c_piix4(E) tpm_tis_core(E) tpm(E) pcspkr(E) button(E) zunicode(POE) zavl(POE) icp(POE) zcommon(POE) znvpair(POE) spl(OE) usbhid(E) sd_mod(E) sr_mod(E) ata_generic(E) virtio_blk(E) 8139too(E) psmouse(E) ahci(E) libahci(E) ehci_pci(E) uhci_hcd(E) ehci_hcd(E) 8139cp(E) virtio_pci(E) mii(E) virtio_ring(E) usbcore(E) ata_piix(E) virtio(E)
[  150.155692] CPU: 1 PID: 2927 Comm: mount.zfs Tainted: P        W  OE   4.12.0-rc1 #1
[  150.155693] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[  150.155694] task: ffff915c839da080 task.stack: ffffba2c43cb8000
[  150.155695] RIP: 0010:sysfs_warn_dup+0x58/0x70
[  150.155696] RSP: 0018:ffffba2c43cbba98 EFLAGS: 00010282
[  150.155697] RAX: 0000000000000042 RBX: ffff915c8f83a000 RCX: 0000000000000000
[  150.155698] RDX: 0000000000000000 RSI: ffff915c9fc4e048 RDI: ffff915c9fc4e048
[  150.155699] RBP: ffffba2c43cbbab0 R08: 0000000000000001 R09: 0000000000000381
[  150.155699] R10: ffff915c9a64a700 R11: 0000000000000381 R12: ffff915c926c6f20
[  150.155700] R13: ffff915bb03161e0 R14: ffff915c8e684c18 R15: ffff915c8d097018
[  150.155701] FS:  00007ff575485780(0000) GS:ffff915c9fc40000(0000) knlGS:0000000000000000
[  150.155702] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  150.155703] CR2: 00007ff5749cfe7f CR3: 000000010d560000 CR4: 00000000000006e0
[  150.155706] Call Trace:
[  150.155708]  sysfs_create_dir_ns+0x77/0x90
[  150.155711]  kobject_add_internal+0xc0/0x2d0
[  150.155712]  kobject_add+0x71/0xd0
[  150.155715]  ? refcount_inc+0x9/0x30
[  150.155717]  device_add+0x117/0x670
[  150.155718]  device_create_groups_vargs+0xe0/0xf0
[  150.155720]  device_create_vargs+0x16/0x20
[  150.155722]  bdi_register_va.part.11+0x2e/0x1b0
[  150.155723]  bdi_register_va+0x1b/0x20
[  150.155725]  super_setup_bdi_name+0x87/0xf0
[  150.155765]  ? dsl_prop_get+0x7a/0xa0 [zfs]
[  150.155795]  zfs_domount+0x121/0x440 [zfs]
[  150.155825]  zpl_fill_super+0x40/0x110 [zfs]
[  150.155853]  ? zpl_statfs+0x110/0x110 [zfs]
[  150.155855]  mount_nodev+0x4e/0xa0
[  150.155882]  zpl_mount+0x34/0x50 [zfs]
[  150.155884]  mount_fs+0x38/0x150
[  150.155886]  vfs_kern_mount+0x64/0x130
[  150.155887]  do_mount+0x1e5/0xcd0
[  150.155889]  ? _copy_from_user+0x33/0x70
[  150.155890]  SyS_mount+0x94/0xd0
[  150.155892]  entry_SYSCALL_64_fastpath+0x1e/0xa9
[  150.155893] RIP: 0033:0x7ff573aa9fda
[  150.155894] RSP: 002b:00007ffec9519fa8 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
[  150.155895] RAX: ffffffffffffffda RBX: 000000000153c060 RCX: 00007ff573aa9fda
[  150.155896] RDX: 00000000004022e6 RSI: 00007ffec951e080 RDI: 00007ffec951fee0
[  150.155896] RBP: 00007ffec9519f90 R08: 00007ffec951b080 R09: 0000000001543650
[  150.155897] R10: 0000000001000000 R11: 0000000000000202 R12: 00007ffec951b080
[  150.155898] R13: 0000000000000005 R14: 0000000000000000 R15: 0000000000000001
[  150.155898] Code: 48 89 c3 74 12 b9 00 10 00 00 48 89 c2 31 f6 4c 89 ef e8 5c c8 ff ff 4c 89 e2 48 89 de 48 c7 c7 68 29 ab 88 31 c0 e8 7f 2c ee ff <0f> ff 48 89 df e8 9e 1a f5 ff 5b 41 5c 41 5d 5d c3 0f 1f 80 00 
[  150.155916] ---[ end trace a2337445839d0b0e ]---
[  150.155918] kobject_add_internal failed for zfs with -EEXIST, don't try to register things with the same name in the same directory.
[  150.155925] ------------[ cut here ]------------
[  150.155928] WARNING: CPU: 1 PID: 2927 at lib/kobject.c:240 kobject_add_internal+0x284/0x2d0
[  150.155928] Modules linked in: zfs(POE) joydev(E) acpi_cpufreq(E) sg(E) evdev(E) pvpanic(E) tpm_tis(E) virtio_balloon(E) virtio_console(E) serio_raw(E) i2c_piix4(E) tpm_tis_core(E) tpm(E) pcspkr(E) button(E) zunicode(POE) zavl(POE) icp(POE) zcommon(POE) znvpair(POE) spl(OE) usbhid(E) sd_mod(E) sr_mod(E) ata_generic(E) virtio_blk(E) 8139too(E) psmouse(E) ahci(E) libahci(E) ehci_pci(E) uhci_hcd(E) ehci_hcd(E) 8139cp(E) virtio_pci(E) mii(E) virtio_ring(E) usbcore(E) ata_piix(E) virtio(E)
[  150.155942] CPU: 1 PID: 2927 Comm: mount.zfs Tainted: P        W  OE   4.12.0-rc1 #1
[  150.155942] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[  150.155943] task: ffff915c839da080 task.stack: ffffba2c43cb8000
[  150.155944] RIP: 0010:kobject_add_internal+0x284/0x2d0
[  150.155945] RSP: 0018:ffffba2c43cbbae8 EFLAGS: 00010282
[  150.155946] RAX: 0000000000000078 RBX: ffff915c8d097010 RCX: 0000000000000006
[  150.155946] RDX: 0000000000000000 RSI: 0000000000000082 RDI: ffff915c9fc4e040
[  150.155947] RBP: ffffba2c43cbbb18 R08: 0000000000000001 R09: 00000000000003b2
[  150.155948] R10: ffff915c9a64a700 R11: 00000000000003b2 R12: 00000000ffffffef
[  150.155948] R13: ffff915bb02f0a80 R14: ffff915c8e684c18 R15: ffff915c8d097018
[  150.155949] FS:  00007ff575485780(0000) GS:ffff915c9fc40000(0000) knlGS:0000000000000000
[  150.155950] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  150.155951] CR2: 00007ff5749cfe7f CR3: 000000010d560000 CR4: 00000000000006e0
[  150.155953] Call Trace:
[  150.155954]  kobject_add+0x71/0xd0
[  150.155956]  ? refcount_inc+0x9/0x30
[  150.155957]  device_add+0x117/0x670
[  150.155959]  device_create_groups_vargs+0xe0/0xf0
[  150.155960]  device_create_vargs+0x16/0x20
[  150.155961]  bdi_register_va.part.11+0x2e/0x1b0
[  150.155962]  bdi_register_va+0x1b/0x20
[  150.155964]  super_setup_bdi_name+0x87/0xf0
[  150.155989]  ? dsl_prop_get+0x7a/0xa0 [zfs]
[  150.156018]  zfs_domount+0x121/0x440 [zfs]
[  150.156046]  zpl_fill_super+0x40/0x110 [zfs]
[  150.156073]  ? zpl_statfs+0x110/0x110 [zfs]
[  150.156075]  mount_nodev+0x4e/0xa0
[  150.156102]  zpl_mount+0x34/0x50 [zfs]
[  150.156104]  mount_fs+0x38/0x150
[  150.156105]  vfs_kern_mount+0x64/0x130
[  150.156106]  do_mount+0x1e5/0xcd0
[  150.156108]  ? _copy_from_user+0x33/0x70
[  150.156109]  SyS_mount+0x94/0xd0
[  150.156111]  entry_SYSCALL_64_fastpath+0x1e/0xa9
[  150.156111] RIP: 0033:0x7ff573aa9fda
[  150.156112] RSP: 002b:00007ffec9519fa8 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
[  150.156113] RAX: ffffffffffffffda RBX: 000000000153c060 RCX: 00007ff573aa9fda
[  150.156114] RDX: 00000000004022e6 RSI: 00007ffec951e080 RDI: 00007ffec951fee0
[  150.156114] RBP: 00007ffec9519f90 R08: 00007ffec951b080 R09: 0000000001543650
[  150.156115] R10: 0000000001000000 R11: 0000000000000202 R12: 00007ffec951b080
[  150.156116] R13: 0000000000000005 R14: 0000000000000000 R15: 0000000000000001
[  150.156116] Code: 7b 20 49 89 c5 48 85 ff 0f 84 3d fe ff ff e9 e4 fd ff ff 48 8b 13 48 c7 c6 00 bd 89 88 48 c7 c7 08 97 af 88 31 c0 e8 03 66 cc ff <0f> ff e9 8f fe ff ff 0f 0b 0f 0b 0f 0b 0f ff eb 9a 0f ff eb 8d 
[  150.156134] ---[ end trace a2337445839d0b0f ]---

Motivation and Context

Fix #6147

How Has This Been Tested?

Manually tested on linux-4.12-rc1

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

@mention-bot
Copy link

@loli10K, thanks for your PR! By analyzing the history of the files in this pull request, we identified @maxximino, @behlendorf and @ahrens to be potential reviewers.


sprintf(tmp, "%.28s%s", name, "-%d");
return super_setup_bdi_name(sb, tmp,
atomic_long_inc_return(&zfs_bdi_seq));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason to use two format functions, fold sprintf into super_setup_bdi_name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tuxoko did you mean something like the following?

super_setup_bdi_name(sb, "%.28s-%ld", name, atomic_long_inc_return(&zfs_bdi_seq));

I basically just copy/pasted the code from https://github.com/zfsonlinux/zfs/blob/5547c2f1bf49802835fd6c52f15115ba344a2a8b/include/linux/vfs_compat.h#L85, should we update that too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should do that. This code is ugly and kinda dangerous, in that if the name is user supplied, then you're using a user supplied format string.

Provide a format parameter to super_setup_bdi_name() so we don't
create duplicate names in '/devices/virtual/bdi' sysfs namespace which
would prevent us from mounting more than one ZFS filesystem at a time.

Signed-off-by: loli10K <[email protected]>
@behlendorf behlendorf merged commit 3e6c943 into openzfs:master May 25, 2017
@loli10K loli10K deleted the issue-6147 branch May 26, 2017 04:36
@loli10K loli10K restored the issue-6147 branch November 1, 2018 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can mount only one filesystem at a time on kernel 4.12.0-rc1
4 participants