Matt,
thank you for looking into this. I will open an issue and a PR. On 20/02/2018 19:25, Matthew Ahrens wrote: > I'm pretty sure I wrote this code, but I can't remember why I thought the > dataset (and objset) couldn't be evicted. I don't think > the dsl_pool_config_enter() prevents that. Your change looks good to me. > > --matt > > On Mon, Feb 19, 2018 at 1:30 AM, Andriy Gapon <[email protected] > <mailto:[email protected]>> wrote: > > > dmu_objset_refresh_ownership() first disowns a dataset (and releases it) > and > then owns it again. There is an assert that the new dataset object is > the same > as the old dataset object: > > dsl_pool_config_enter(dp, FTAG); > dmu_objset_disown(os, tag); > VERIFY0(dsl_dataset_own(dp, name, tag, &newds)); > ==> VERIFY3P(newds, ==, os->os_dsl_dataset); > dsl_pool_config_exit(dp, FTAG); > > I cannot see what can really guarantee that assert to be true. > I think that the dataset object is allowed to be evicted, so it's possible > that > dsl_dataset_own() -> dsl_dataset_hold() may need to create a new object. > Maybe I am missing something here. Perhaps what's being asserted used to be > true before some changes in dbuf / dataset eviction.. > > When running ZFS Test Suite on FreeBSD we see this panic from > zpool_upgrade_007_pos test: > > panic: solaris assert: newds == os->os_dsl_dataset (0xfffff80045f4c000 == > 0xfffff80021ab4800) > > assfail3() at assfail3+0x2c/frame 0xfffffe002a621480 > dmu_objset_refresh_ownership() at dmu_objset_refresh_ownership+0x161/frame > 0xfffffe002a6215c0 > zfs_ioc_userspace_upgrade() at zfs_ioc_userspace_upgrade+0x97/frame > 0xfffffe002a621600 > zfs_prop_set_special() at zfs_prop_set_special+0x465/frame 0xfffffe002a621670 > zfs_set_prop_nvlist() at zfs_set_prop_nvlist+0x23f/frame 0xfffffe002a6216f0 > zfs_ioc_set_prop() at zfs_ioc_set_prop+0x129/frame 0xfffffe002a621740 > zfsdev_ioctl() at zfsdev_ioctl+0x76b/frame 0xfffffe002a6217e0 > > I see that the old dataset has dsl_dataset_evict_async() pending in > ds_dbu.dbu_tqent and its ds_dbuf is NULL. > > I've got this patch to fix the problem: > https://paste.debian.net/1010940/ <https://paste.debian.net/1010940/> > > What do you think? > Thank you! > -- > Andriy Gapon > > *openzfs-developer* | Archives > <https://openzfs.topicbox.com/groups/developer/discussions/T9d8097b8fffd8bfa-Mbf114e3ce50bbb0fe2db7ca6> > | Powered by Topicbox <https://topicbox.com> -- Andriy Gapon ------------------------------------------ openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T9d8097b8fffd8bfa-Mf731f816f21b610595e49651 Powered by Topicbox: https://topicbox.com
