Skip to content

Commit

Permalink
Wait for all znodes to be released before tearing down the superblock
Browse files Browse the repository at this point in the history 
By the time we're tearing down our superblock the VFS has started releasing
all our inodes/znodes. Some of this work may have been handed off to our
iput taskq so we need to wait for that work to complete. However the iput
from the taskq can itself result in additional work being added to the
taskq:

dsl_pool_iput_taskq
 iput
  iput_final
   evict
    destroy_inode
     zpl_inode_destroy
      zfs_inode_destroy
       zfs_iput_async(ZTOI(zp->z_xattr_parent))
        taskq_dispatch(dsl_pool_iput_taskq..., iput, ...)

Let's wait until all our znodes have been released.

Signed-off-by: Chris Dunlop <[email protected]>
Closes openzfs#3281
  • Loading branch information
@chrisrd
chrisrd committed Apr 17, 2015
1 parent b467db4 commit a8f8e2d
Showing 1 changed file with 9 additions and 19 deletions.
28 changes: 9 additions & 19 deletions module/zfs/zfs_vfsops.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1130,15 +1130,20 @@ EXPORT_SYMBOL(zfs_sb_prune);
int
zfs_sb_teardown(zfs_sb_t *zsb, boolean_t unmounting)
{
znode_t *zp;

/*
* If someone has not already unmounted this file system,
* drain the iput_taskq to ensure all active references to the
* zfs_sb_t have been handled only then can it be safely destroyed.
*
* We can safely read z_nr_znodes without locking because the VFS
* has already blocked operations which add to the z_all_znodes
* list and increment z_nr_znodes.
*/
if (zsb->z_os)
taskq_wait(dsl_pool_iput_taskq(dmu_objset_pool(zsb->z_os)));
while (zsb->z_nr_znodes) {
schedule();
taskq_wait(dsl_pool_iput_taskq(dmu_objset_pool(
zsb->z_os)));
}

This comment has been minimized.

Sign in to view

Copy link
@behlendorf

behlendorf Apr 23, 2015

Yes, this is very close to what I had in mind, just a few comments.

  • We're only 100% guaranteed that zsb->z_nr_znodes will drop to 0 in the unmounted == TRUE case. For something like a rollback or a zfs receive there could potentially still be znodes pinned by open files.
  • There's no need to explicitly call schedule() here.
  • For readability and clarity you may want to use something like while (zsb->z_nr_znodes > 0) { }

rrw_enter(&zsb->z_teardown_lock, RW_WRITER, FTAG);

Expand Down Expand Up @@ -1175,21 +1180,6 @@ zfs_sb_teardown(zfs_sb_t *zsb, boolean_t unmounting)
return (SET_ERROR(EIO));
}

/*
* At this point there are no VFS ops active, and any new VFS ops
* will fail with EIO since we have z_teardown_lock for writer (only
* relevant for forced unmount).
*
* Release all holds on dbufs.
*/
mutex_enter(&zsb->z_znodes_lock);
for (zp = list_head(&zsb->z_all_znodes); zp != NULL;
zp = list_next(&zsb->z_all_znodes, zp)) {
if (zp->z_sa_hdl)
zfs_znode_dmu_fini(zp);
}
mutex_exit(&zsb->z_znodes_lock);

This comment has been minimized.

Sign in to view

Copy link
@behlendorf

behlendorf Apr 23, 2015

Rather than drop this hunk entirely lets convert it to an ASSERT(list_empty(&zsb->z_all_znodes)) and move it in to the unmounting block below.
/*
* If we are unmounting, set the unmounted flag and let new VFS ops
* unblock. zfs_inactive will have the unmounted behavior, and all
Expand Down

0 comments on commit a8f8e2d

Please sign in to comment.