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

Metadata Allocation Classes #676

Merged
merged 6 commits into from
Jan 31, 2019
Merged

Metadata Allocation Classes #676

merged 6 commits into from
Jan 31, 2019

Conversation

lundman
Copy link
Contributor

@lundman lundman commented Dec 27, 2018

This is the OSX version of: openzfs/zfs#5182

It was easier to include the "status_cbdata_t" changes to prints as well, and we could consider ZOL 8720e9e7482fa2d Add -c to zpool iostat & status to run command.

@lundman lundman changed the title Metadata Allocation Classe Metadata Allocation Classes Dec 27, 2018
@JMoVS
Copy link
Contributor

JMoVS commented Dec 27, 2018

does this need testing? And could you maybe tag PRs that need testing with a label à la "needs testing"? Then I can try to have a look

@lundman
Copy link
Contributor Author

lundman commented Jan 29, 2019

FAIL	 223
PASS	 645

Running Time:	03:23:56
Percent passed:	74.3%

So this passes, looks good for merge.

inkdot7 and others added 3 commits January 30, 2019 17:17
First rename spare_cbdata_t cb -> spare_cb in print_status_config(),
to free up cb.

Using the structure removes the explicit parameters namewidth
and name_flags from several functions.  Also use status_cbdata_t
for print_import_config().  This simplifies print_logs().

Remove the parameter 'verbose' for print_logs().  It does not really
mean verbose, it selected between the print_status_config and
print_import_config() paths.  This selection is now done by
cb_print_config of spare_cbdata_t.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Håkan Johansson <f96hajo@chalmers.se>
Allocation Classes add the ability to have allocation classes in a
pool that are dedicated to serving specific block categories, such
as DDT data, metadata, and small file blocks. A pool can opt-in to
this feature by adding a 'special' or 'dedup' top-level VDEV.

Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed-by: Richard Laager <rlaager@wiktel.com>
Reviewed-by: Alek Pinchuk <apinchuk@datto.com>
Reviewed-by: Håkan Johansson <f96hajo@chalmers.se>
Reviewed-by: Andreas Dilger <andreas.dilger@chamcloud.com>
Reviewed-by: DHE <git@dehacked.net>
Reviewed-by: Richard Elling <Richard.Elling@RichardElling.com>
Reviewed-by: Gregor Kopka <gregor@kopka.net>
Reviewed-by: Kash Pande <kash@tripleback.net>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Matthew Ahrens <mahrens@delphix.com>
Signed-off-by: Don Brady <don.brady@delphix.com>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Richard Laager <rlaager@wiktel.com>
Signed-off-by: DHE <git@dehacked.net>
@lundman
Copy link
Contributor Author

lundman commented Jan 30, 2019

The actual branch:

FAIL	 231
PASS	 637

Running Time:	03:06:49
Percent passed:	73.4%

@lundman
Copy link
Contributor Author

lundman commented Jan 31, 2019

The alloc_class test scripts needed change truncate to $TRUNCATE.

Panic in alloc_class_006

panic(cpu 1 caller 0xffffff7f842c55eb): "trying to interlock destroyed mutex (0xffffff90a7aebd90)"@/BuildRoot/Library/Caches/com.apple.xbs/Sources/xnu/xnu-4903.240.8/osfmk/i386/locks_i386.c:2817
Backtrace (CPU 1), Frame : Return Address
0xffffff90af37b910 : 0xffffff8001c8601d mach_kernel : _handle_debugger_trap + 0x47d
0xffffff90af37b960 : 0xffffff8001e22cc7 mach_kernel : _kdp_i386_trap + 0x197
0xffffff90af37b9a0 : 0xffffff8001e125f2 mach_kernel : _kernel_trap + 0x3c2
0xffffff90af37ba10 : 0xffffff8001c21ca0 mach_kernel : trap_from_kernel + 0x26
0xffffff90af37ba30 : 0xffffff8001c856d9 mach_kernel : _panic_trap_to_debugger + 0x1b9
0xffffff90af37bb60 : 0xffffff8001c85503 mach_kernel : _panic + 0x63
0xffffff90af37bbd0 : 0xffffff7f842c55eb net.lundman.spl : _spl_mutex_enter + 0x4b
0xffffff90af37bbf0 : 0xffffff7f855466a8 net.lundman.zfs : _vdev_dtl_sync + 0x228
0xffffff90af37bc50 : 0xffffff7f855471eb net.lundman.zfs : _vdev_sync + 0xeb
0xffffff90af37bca0 : 0xffffff7f85532dfb net.lundman.zfs : _spa_sync + 0xb2b
0xffffff90af37bee0 : 0xffffff7f855409f7 net.lundman.zfs : _txg_sync_thread + 0x337

Caused by

frame #6: 0xffffff7f8f727668 zfs`vdev_dtl_sync(vd=0xffffff90a94fa000, txg=11) at vdev.c:2742 [opt]
   2739
   2740		rtsync = range_tree_create(NULL, NULL);
   2741
-> 2742		mutex_enter(&vd->vdev_dtl_lock);
   2743		range_tree_walk(rt, range_tree_add, rtsync);
   2744		mutex_exit(&vd->vdev_dtl_lock);

Mutex vdev_dtl_lock destroyed at line 899 in vdev_free()

(lldb) p vd
(vdev_t *) $1 = 0xffffff90c8e7a000
# dtrace -n 'vdev_free:entry { printf("%p", arg0); stack(); }'

  0  73807                  vdev_free:entry ffffff90c8e7a000
              zfs`spa_vdev_split_mirror+0xb9b
              zfs`zfs_ioc_vdev_split+0x1ac
              zfs`zfsdev_ioctl+0x51c
              kernel.development`spec_ioctl+0xaa
              kernel.development`VNOP_IOCTL+0xbf
              kernel.development`vn_ioctl+0x1be
              kernel.development`fo_ioctl+0x7b
              kernel.development`ioctl+0x52c
              kernel.development`unix_syscall64+0x2f8
              kernel.development`hndl_unix_scall64+0x16

# (lldb) image lookup --verbose --address spa_vdev_split_mirror+0xb9b
    LineEntry: [0xffffff7faee90c5b-0xffffff7faee90c5e): /Users/lundman/src/zfs/osx.zfs/x/zfs/module/zfs/spa.c:7016:18

				spa_history_log_internal(spa, "detach", tx,
				    "vdev=%s", vml[c]->vdev_path);

			vdev_free(vml[c]);

Ok, repeated executing of
zpool create -f pool mirror disk1 disk2 ; zpool split pool split_pool; zpool destroy pool will trigger it without special devices, so a preexisting condition.

@lundman
Copy link
Contributor Author

lundman commented Jan 31, 2019

001 test has

log_must zpool create -f $TESTPOOL raidz $ZPOOL_DISKS special mirror \
    $CLASS_DISK0 $CLASS_DISK1

causes

09:26:27.15 invalid vdev specification use '-f' to override the following errors: 
     mismatched replication level: both raidz and mirror vdevs are present

Is it due to mis-merge or that we have illumos commits.

@lundman
Copy link
Contributor Author

lundman commented Jan 31, 2019

With the exception of 006, and I had to comment out zdb -bbcc for some reason, the tests PASS:

Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/setup (run as root) [00:00] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_001_pos (run as root) [00:03] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_002_neg (run as root) [00:06] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_003_pos (run as root) [00:01] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_004_pos (run as root) [00:01] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_005_pos (run as root) [00:01] [PASS]

Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_007_pos (run as root) [00:10] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_008_pos (run as root) [00:01] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_009_pos (run as root) [00:10] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_010_pos (run as root) [00:01] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_011_neg (run as root) [00:00] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_012_pos (run as root) [00:19] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/alloc_class_013_pos (run as root) [00:15] [PASS]
Test: /Users/lundman/src/zfs/osx.zfs/x/zfs/tests/zfs-tests/tests/functional/alloc_class/cleanup (run as root) [00:00] [PASS]

Adding $NAWK and $TRUNCATE. Curious our ksh does not like the space in
claim= ""

Our zdb -bbcc abort()s.

Many create and add require "-f" for the mixing of raidz and mirror check.
@lundman
Copy link
Contributor Author

lundman commented Jan 31, 2019

@don-brady If you have a minute sometime, could you give your thought on the panic in 006 tester (the zpool split). I have tested that zpool split works without special devices.

If you also want, you can look at 78fb6dd fixes for test files. I don't think you need to do anything about it, but it could be something to be aware of in future tests files you create :)

@lundman
Copy link
Contributor Author

lundman commented Jan 31, 2019

I would seem Solaris and ZOL has touched on this issue: openzfs/zfs#7856

But it is not fixed with ZOLs openzfs/zfs@733b572

Repeatedly running

# zpool create -f pool mirror disk1 disk0 ; zpool split pool split_pool; zpool destroy pool

is enough to trigger here, about ~20 times.

ramzec and others added 2 commits January 31, 2019 11:36
Added vdev_resilver_needed() check to verify VDEVs are fully
synced, so that after split the new pool will not be corrupted.

Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: loli10K <ezomori.nozomu@gmail.com>
Signed-off-by: Roman Strashkin <roman.strashkin@nexenta.com>
Added missing remove of detachable VDEV from txg's DTL list
to avoid use-after-free for the splitted VDEV

Signed-off-by: Roman Strashkin <roman.strashkin@nexenta.com>
@lundman
Copy link
Contributor Author

lundman commented Jan 31, 2019

After applying ZOL openzfs/zfs@cec8583 from openzfs/zfs#7856

The panics go away:

for i in {1..100}; do zpool create -f pool mirror disk1 disk0 ; zpool split pool split_pool; zpool destroy pool ; sleep 1; done

We will take this commit, with thanks :)

@lundman lundman merged commit d48d874 into master Jan 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants