Skip to content

Commit

Permalink
FreeBSD: Clean up zfsdev_close to match Linux
Browse files Browse the repository at this point in the history
Resolve some oddities in zfsdev_close() which could result in a
panic and were not present in the equivalent function for Linux.

- Remove unused definition ZFS_MIN_MINOR
- FreeBSD: Simplify zfsdev state destruction
- Assert zs_minor is valid in zfsdev_close
- Make locking around zfsdev state match Linux

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes #11720
  • Loading branch information
Ryan Moeller authored Mar 13, 2021
1 parent e3e82dc commit f845b2d
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 11 deletions.
1 change: 0 additions & 1 deletion include/sys/zfs_ioctl.h
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,6 @@ typedef struct zfs_useracct {
} zfs_useracct_t;

#define ZFSDEV_MAX_MINOR (1 << 16)
#define ZFS_MIN_MINOR (ZFSDEV_MAX_MINOR + 1)

#define ZPOOL_EXPORT_AFTER_SPLIT 0x1

Expand Down
18 changes: 8 additions & 10 deletions module/os/freebsd/zfs/kmod_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,23 +182,21 @@ zfsdev_ioctl(struct cdev *dev, ulong_t zcmd, caddr_t arg, int flag,
static void
zfsdev_close(void *data)
{
zfsdev_state_t *zs, *zsp = data;
zfsdev_state_t *zs = data;

ASSERT(zs != NULL);

mutex_enter(&zfsdev_state_lock);
for (zs = zfsdev_state_list; zs != NULL; zs = zs->zs_next) {
if (zs == zsp)
break;
}
if (zs == NULL || zs->zs_minor <= 0) {
mutex_exit(&zfsdev_state_lock);
return;
}

ASSERT(zs->zs_minor != 0);

zs->zs_minor = -1;
zfs_onexit_destroy(zs->zs_onexit);
zfs_zevent_destroy(zs->zs_zevent);
mutex_exit(&zfsdev_state_lock);
zs->zs_onexit = NULL;
zs->zs_zevent = NULL;

mutex_exit(&zfsdev_state_lock);

This comment has been minimized.

Copy link
@grembo

grembo Jan 2, 2022

Contributor

@freqlabs Is moving the lock (mutex_exit) the change that fixed https://jira.ixsystems.com/browse/NAS-108891? I'm seeing a panic that looks exactly like what is described in the bug on FreeBSD 13.0-RELEASE/OpenZFS 2.0.0.

This comment has been minimized.

Copy link
@grembo

grembo Jan 2, 2022

Contributor

To answer my own question: Seems like this is the case, I was able to reproduce the kernel panic reliably and applying this commit corrected it.

I created a PR in FreeBSD's bug tracker documenting the problem and the solution for 13.0-RELEASE.

}

static int
Expand Down

0 comments on commit f845b2d

Please sign in to comment.