I'm experiencing lock-up of the ZFS system when doing a specific command sequence. It is reproducible. pool structure: tank/vol/vol1@1 tank/vol/vol2@1 tank/grp/grp1@a, b, c tank/grp/grp2@a, b, c Any object under /vol/ and /grp/ are ZVOLs, grp's are clones vol@1. The commands: zfs rename tank/vol/vol1@1 tank/vol/vol1@YYYY-MM-DD zfs rename tank/vol/vol1 tank/trash/YYYY-MM-DD_vol1 zfs rename tank/grp/grp1 tank/vol/vol1 zfs promote tank/vol/vol1 *hangs* next command: zfs snapshot tank/vol/vol@1 I can't use 'zfs list' during this time, it is hung: 82867 - I 0:00.00 sh -c zfs ... 82870 - I 0:00.01 sudo zfs promote tank/iscsi/volumes/dfh 82871 - D 0:00.01 zfs promote tank/iscsi/volumes/dfh 82919 - I 0:00.01 sudo zfs get -H -o value available 82920 - D 0:00.00 zfs get -H -o value available
Hi, When the system is in a hung state, can you please show the output of ps -auxwww -O wchan |grep zfs And for each process listed, run "procstat -kk [pid]" It would also be interesting to know if there is any disk activity while in the hung state. "iostat -t da -c 5 5" should do it. Thanks, Gavin
It appears this is not dependent on 'promote' being used, I just hit the bug again when running 10 rename commands very closely together. Thing is, it did not crash the first three times I ran the commands, but it did crash on the fourth. This is a trend I have observed. After a reboot, the commands will run, but if you do it too many times, it will stop working. (In reply to Gavin Atkinson from comment #1) > Hi, > > When the system is in a hung state, can you please show the output of > > ps -auxwww -O wchan |grep zfs > > And for each process listed, run "procstat -kk [pid]" > > It would also be interesting to know if there is any disk activity while in > the hung state. "iostat -t da -c 5 5" should do it. > > Thanks, > > Gavin
ps auxwww -O wchan: root 2 0.0 0.0 0 1424 - DL Mon09am 38:58.35 [zfskern] 2 zvol:io - DL 38:58.35 [zfskern] root 5452 0.0 0.0 48152 2604 - I 2:03pm 0:00.01 sudo zfs rename 5452 select - I 0:00.01 sudo zfs rename tank/[redacted]@b7 tank/[redacted]@b6 root 5467 0.0 0.0 40048 2448 - D 2:03pm 0:00.01 zfs rename tank/ 5467 tx->tx_s - D 0:00.01 zfs rename tank/[redacted]@b7 tank/[redacted]@b6 root 5474 0.0 0.0 18724 2108 0 S+ 2:05pm 0:00.00 grep zfs 5474 piperd 0 S+ 0:00.00 grep zfs # for i in 5452 5467 ; do procstat -kk $i ; done PID TID COMM TDNAME KSTACK 5452 101591 sudo - mi_switch+0xde sleepq_catch_signals+0xb2 sleepq_wait_sig+0xf _cv_wait_sig+0x16a seltdwait+0x96 sys_poll+0x3dc amd64_syscall+0x357 Xfast_syscall+0xfb PID TID COMM TDNAME KSTACK 5467 101597 zfs - mi_switch+0xde sleepq_wait+0x3a _cv_wait+0x16d txg_wait_synced+0x85 dsl_sync_task+0x1e2 dsl_dataset_rename_snapshot+0x34 zfs_ioc_rename+0x10e zfsdev_ioctl+0x478 devfs_ioctl_f+0x11f kern_ioctl+0x22e sys_ioctl+0x11f amd64_syscall+0x357 Xfast_syscall+0xfb Looks like no disk IO: # iostat -t da -c 5 5 tty ada0 ada1 cpu tin tout KB/t tps MB/s KB/t tps MB/s us ni sy in id 0 58 18.25 71 1.27 18.50 70 1.27 0 0 2 0 98 0 37 4.00 0 0.00 4.00 0 0.00 0 0 0 0 100 0 12 4.00 0 0.00 0.00 0 0.00 0 0 0 0 100 0 12 0.00 0 0.00 0.00 0 0.00 0 0 0 0 100 0 12 0.00 0 0.00 0.00 0 0.00 0 0 0 0 100 0 12 0.00 0 0.00 0.00 0 0.00 0 0 0 0 100 0 12 0.00 0 0.00 0.00 0 0.00 0 0 0 0 100 0 12 0.00 0 0.00 0.00 0 0.00 0 0 0 0 100
I've been hitting this bug repeatedly on production networks, it would be best to get some feedback or it starts to look like FreeBSD ZFS is a dead project.
This bug along with the "zvol entries not showing up in /dev/ until after reboot" one, and the lack of response shows the FreeBSD developers have different priorities than the users, BHyVE gets who knows how much unnecessary attention while critical ZFS bugs go unfixed? The proposed patch for #178999 in the old tracker works but doesn't resolve all the issues, even admitted by the author smh@freebsd, but yet, no additional work has gone in that ticket. It's been "merged into head" but the new servers I've built still have the issue. ZFS on Linux does not have the issues that FreeBSD ZFS has, so unfortunately my company and our product (plus its thousands of users) will be converting to a more reliable platform until some progress can be made here.
Created attachment 147905 [details] Possible fix for zvol hang issues Try this patch and lmk if it fixes the issues for you
Comment on attachment 147905 [details] Possible fix for zvol hang issues This does not compile against 10-RELEASE-pX
(In reply to kash from comment #7) > Comment on attachment 147905 [details] > Possible fix for zvol hang issues > > This does not compile against 10-RELEASE-pX Its against stable/10 not release
Can I get a patch for my RELEASE, because that's the version I filed this bug against, I don't run STABLE and can't jump to a test version just for one bug.. thanks
Created attachment 147915 [details] Possible fix for zvol hang issues (against 10.0-RELEASE)
--- dnode_sync.o --- ctfconvert -L VERSION -g dnode_sync.o --- dnode.o --- ctfconvert -L VERSION -g dnode.o --- dsl_dataset.o --- /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c:3016:12: error: use of undeclared identifier 'newname' kmem_free(newname, MAXPATHLEN); ^ /usr/src/sys/modules/zfs/../../cddl/compat/opensolaris/sys/kmem.h:82:46: note: expanded from macro 'kmem_free' #define kmem_free(buf, size) zfs_kmem_free((buf), (size)) ^ /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c:3017:12: error: use of undeclared identifier 'oldname' kmem_free(oldname, MAXPATHLEN); ^ /usr/src/sys/modules/zfs/../../cddl/compat/opensolaris/sys/kmem.h:82:46: note: expanded from macro 'kmem_free' #define kmem_free(buf, size) zfs_kmem_free((buf), (size)) ^ 2 errors generated. *** [dsl_dataset.o] Error code 1
(In reply to kash from comment #11) > --- dnode_sync.o --- > ctfconvert -L VERSION -g dnode_sync.o > --- dnode.o --- > ctfconvert -L VERSION -g dnode.o > --- dsl_dataset.o --- > /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/ > dsl_dataset.c:3016:12: error: use of undeclared identifier 'newname' > kmem_free(newname, MAXPATHLEN); ... Line 3016 in 10.0-RELEASE is in dsl_dataset_space_wouldfree which has no changes in this patch, are you sure it applied cleanly? kmem_free(newname, MAXPATHLEN) should be on the following two lines only: "sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c" line 1696 of 3073 --55%-- col 2-9 "sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c" line 2229 of 3073 --72%-- col 2-9
In addition the functions these calls are in are: dsl_dataset_promote_sync and dsl_dataset_rename_snapshot_sync_impl with the new one being added to: dsl_dataset_promote_sync
Ah, I'm using RELEASE-p7 it seems.
-pX is just security patches, that won't effect the main code base.
The patch does not apply cleanly to a new SVN pull of releng/10.0: [/usr/src]-[root@fbsd-master]-[0]-[1704] [:)] # cat sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c.rej @@ -1684,8 +1682,7 @@ VERIFY0(zap_add(dp->dp_meta_objset, hds->ds_phys->ds_snapnames_zapobj, ds->ds_snapname, 8, 1, &ds->ds_object, tx)); -#ifdef __FreeBSD__ -#ifdef _KERNEL +#if defined(__FreeBSD__) && defined (_KERNEL) oldname = kmem_alloc(MAXPATHLEN, KM_SLEEP); newname = kmem_alloc(MAXPATHLEN, KM_SLEEP); snprintf(oldname, MAXPATHLEN, "%s@%s", ddrsa->ddrsa_fsname, @@ -2144,6 +2143,14 @@ dd->dd_phys->dd_clones, origin_head->ds_object, tx)); } +#if defined(__FreeBSD__) && defined(_KERNEL) + /* Take the spa_namespace_lock so zvol renames don't livelock */ + mutex_enter(&spa_namespace_lock); + + oldname = kmem_alloc(MAXPATHLEN, KM_SLEEP); + newname = kmem_alloc(MAXPATHLEN, KM_SLEEP); +#endif + /* move snapshots to this dir */ for (snap = list_head(&ddpa->shared_snaps); snap; snap = list_next(&ddpa->shared_snaps, snap)) { [/usr/src]-[root@fbsd-master]-[0]-[1705] [:)] # cat sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c.rej @@ -2393,9 +2381,10 @@ if (dmu_objset_type(os) == DMU_OST_ZVOL) { dsl_dataset_long_hold(os->os_dsl_dataset, FTAG); dsl_pool_rele(dmu_objset_pool(os), FTAG); - if ((error = zvol_create_minor(name)) == 0) + error = zvol_create_minor(name); + if (error == 0 || error == EEXIST) { error = zvol_create_snapshots(os, name); - else { + } else { printf("ZFS WARNING: Unable to create ZVOL %s (error=%d).\n", name, error); } @@ -2479,12 +2468,16 @@ size_t oldnamelen, newnamelen; zvol_state_t *zv; char *namebuf; + boolean_t locked = B_FALSE; oldnamelen = strlen(oldname); newnamelen = strlen(newname); DROP_GIANT(); - mutex_enter(&spa_namespace_lock); + if (!MUTEX_HELD(&spa_namespace_lock)) { + mutex_enter(&spa_namespace_lock); + locked = B_TRUE; + } g_topology_lock(); LIST_FOREACH(gp, &zfs_zvol_class.geom, geom) { @@ -2507,6 +2500,7 @@ } g_topology_unlock(); - mutex_exit(&spa_namespace_lock); + if (locked) + mutex_exit(&spa_namespace_lock); PICKUP_GIANT(); }
(In reply to kash from comment #16) > The patch does not apply cleanly to a new SVN pull of releng/10.0: Just retested here against a clean checkout and it does apply for me without issue. Are you sure your checkout is clean? smh> svn info Path: . Working Copy Root Path: freebsd/base/releng/10.0 URL: svn+ssh://svn.freebsd.org/base/releng/10.0 Relative URL: ^/releng/10.0 Repository Root: svn+ssh://svn.freebsd.org/base Repository UUID: ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f Revision: 272442 Node Kind: directory Schedule: normal Last Changed Author: delphij Last Changed Rev: 271669 Last Changed Date: 2014-09-16 09:50:19 +0000 (Tue, 16 Sep 2014) smh> svn status smh> root@node-dev:src-svn> patch -C -E -N -p0 -F1 -t < /usr/src/patches/zfs-zvol-fixes.patch Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |Various fixes for zvol devices not being correctly maintained when operations |which effect them occur. |--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c.orig 2014-03-26 17:52:33.000000000 +0000 |+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c 2014-09-29 18:52:24.903520480 +0000 -------------------------- Patching file sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c using Plan A... Hunk #1 succeeded at 1655. Hunk #2 succeeded at 1684. Hunk #3 succeeded at 1696. Hunk #4 succeeded at 2072. Hunk #5 succeeded at 2140. Hunk #6 succeeded at 2180. Hunk #7 succeeded at 2223. Hmm... The next patch looks like a unified diff to me... The text leading up to this was: -------------------------- |--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zvol.h.orig 2014-03-26 17:52:33.000000000 +0000 |+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zvol.h 2014-09-29 18:52:24.903520480 +0000 -------------------------- Patching file sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zvol.h using Plan A... Hunk #1 succeeded at 41. Hmm... The next patch looks like a unified diff to me... The text leading up to this was: -------------------------- |--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c.orig 2014-03-26 17:52:33.000000000 +0000 |+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c 2014-09-29 18:52:24.908521871 +0000 -------------------------- Patching file sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c using Plan A... Hunk #1 succeeded at 3517. Hunk #2 succeeded at 3547. Hunk #3 succeeded at 3643. Hmm... The next patch looks like a unified diff to me... The text leading up to this was: -------------------------- |--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c.orig 2014-09-29 18:52:05.348521778 +0000 |+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c 2014-09-29 18:52:24.909520935 +0000 -------------------------- Patching file sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c using Plan A... Hunk #1 succeeded at 630. Hunk #2 succeeded at 757. Hunk #3 succeeded at 782. Hunk #4 succeeded at 2369 (offset -12 lines). Hunk #5 succeeded at 2467 (offset -1 lines). No such line 2505 in input file, ignoring Hunk #6 succeeded at 2488 (offset -12 lines). done
I've built it but now I don't have /dev/zvol nodes showing up for clones.
/usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c:1126:11: warning: comparison of constant -1 with expression of type 'zfs_prop_t' is always false [-Wtautological-constant-out-of-range-compare] if (prop == ZPROP_INVAL) { ~~~~ ^ ~~~~~~~~~~~ /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c:2378:11: warning: comparison of constant -1 with expression of type 'zfs_prop_t' is always false [-Wtautological-constant-out-of-range-compare] if (prop == ZPROP_INVAL) { ~~~~ ^ ~~~~~~~~~~~ /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c:2512:24: warning: comparison of constant -1 with expression of type 'zfs_prop_t' is always false [-Wtautological-constant-out-of-range-compare] if (err == 0 && prop == ZPROP_INVAL) { ~~~~ ^ ~~~~~~~~~~~ /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c:2774:12: warning: comparison of constant -1 with expression of type 'zfs_prop_t' is always false [-Wtautological-constant-out-of-range-compare] if (prop == ZPROP_INVAL) { ~~~~ ^ ~~~~~~~~~~~ /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c:2813:12: warning: comparison of constant -1 with expression of type 'zfs_prop_t' is always true [-Wtautological-constant-out-of-range-compare] if (prop != ZPROP_INVAL && !zfs_prop_inheritable(prop)) ~~~~ ^ ~~~~~~~~~~~ /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c:3661:11: warning: comparison of constant -1 with expression of type 'zfs_prop_t' is always false [-Wtautological-constant-out-of-range-compare] if (prop == ZPROP_INVAL) {
(In reply to kash from comment #19) > /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/ > zfs_ioctl.c:1126:11: warning: comparison of constant -1 with expression of > type 'zfs_prop_t' is always false > [-Wtautological-constant-out-of-range-compare] > if (prop == ZPROP_INVAL) { > ~~~~ ^ ~~~~~~~~~~~ Nothing to do with this, it always does that.
Ok, wasn't sure if that was the intended behaviour - was zvol dev entry creation purposefully removed?
(In reply to kash from comment #18) > I've built it but now I don't have /dev/zvol nodes showing up for clones. That's an issue already fixed in stable, its never worked in 10.0-RELEASE
I've been using 10.0-RELEASE for a year with the original patch from this thread. It does work, but not with the new locking patch you created.
Created attachment 147924 [details] Possible fix for zvol hang issues (against 10.0-RELEASE) This also includes the fix from stable for clones not creating their corrisponding zvols.
(In reply to kash from comment #23) > I've been using 10.0-RELEASE for a year with the original patch from this > thread. It does work, but not with the new locking patch you created. There's no other patches on this PR, so I'm not sure what your refering to but I've updated the patch for 10.0-RELEASE adding the fix from stable for clones creating zvols. I'd appreciate feedback on if the patch fixes your deadlock issue.
(In reply to Steven Hartland from comment #25) > (In reply to kash from comment #23) > > I've been using 10.0-RELEASE for a year with the original patch from this > > thread. It does work, but not with the new locking patch you created. > > There's no other patches on this PR, so I'm not sure what your refering to > but I've updated the patch for 10.0-RELEASE adding the fix from stable for > clones creating zvols. > > I'd appreciate feedback on if the patch fixes your deadlock issue. I meant from the other 'zvol not appearing' ticket.
Thanks for your patience, that's a good enough fix for now, hopefully soon the locking code can be reworked within OpenZFS to have a generic approach across Linux, Illumos and FreeBSD.
So just to confirm, this does fix your deadlock issue? For reference the current locking for zvol changes is actually quite specific to FreeBSD so not really an upstream issue.
yes so far it resolves rename-related deadlocks.
(In reply to Steven Hartland from comment #28) > So just to confirm, this does fix your deadlock issue? > > For reference the current locking for zvol changes is actually quite > specific to FreeBSD so not really an upstream issue. Richard Yao was saying there's potential for converging the pathways still, ZoL and Illumos have different ways of structuring locks, and thought the way this was handled is a bit too similar to the Big Kernel Lock, but I don't know much about this, I'm a less articulate coder.
(In reply to kash from comment #30) > (In reply to Steven Hartland from comment #28) > > So just to confirm, this does fix your deadlock issue? > > > > For reference the current locking for zvol changes is actually quite > > specific to FreeBSD so not really an upstream issue. > > Richard Yao was saying there's potential for converging the pathways still, > ZoL and Illumos have different ways of structuring locks, and thought the > way this was handled is a bit too similar to the Big Kernel Lock, but I > don't know much about this, I'm a less articulate coder. I believe this is around FS locking. The reason for the divergence in the code bases around ZVOLs is due to the fact that upstream creates ZVOL dev entries on the demand when a /dev/ lookup takes place. This is something FreeBSD currently can't do hence the various static create/destroy/rename in various places so they are always available.
A commit references this bug: Author: smh Date: Fri Oct 3 14:49:50 UTC 2014 New revision: 272474 URL: https://svnweb.freebsd.org/changeset/base/272474 Log: Fix various issues with zvols When performing snapshot renames we could deadlock due to the locking in zvol_rename_minors. In order to avoid this use the same workaround as zvol_open in zvol_rename_minors. Add missing zvol_rename_minors to dsl_dataset_promote_sync. Protect against invalid index into zv_name in zvol_remove_minors. Replace zvol_remove_minor calls with zvol_remove_minors to ensure any potential children are also renamed. Don't fail zvol_create_minors if zvol_create_minor returns EEXIST. Restore the valid pool check in zfs_ioc_destroy_snaps to ensure we don't call zvol_remove_minors when zfs_unmount_snap fails. PR: 193803 MFC after: 1 week Sponsored by: Multiplay Changes: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
This is still an issue - whatever commits were merged did not fully address the situation
(In reply to kash from comment #33) Please confirm exactly what revision your running and report an updated stack. The fix was included in r277483 for stable/10
To originators/assignees of this PR: A commit to the tree references this PR, however the PR is still in a non-closed state. Please review this PR and close as appropriate, or if closing the PR requires a merge to stable/10, please let re@ know as soon as possible. Thank you. Glen
Created attachment 161674 [details] deadlocking testcase
Unfortunately deadlock can still occur running the attached deadlocking testcase script.