Page MenuHomeFreeBSD

zfs_mount(MS_REMOUNT): protect zfs_(un)register_callbacks calls
ClosedPublic

Authored by avg on Jun 19 2015, 7:30 AM.

Details

Summary

We now take z_teardown_lock as a writer to ensure that there is no
I/O while the filesystem state is in a flux.
Also, zfs_suspend_fs() -> zfsvfs_teardown() call zfs_unregister_callbacks()
and zfs_resume_fs() -> zfsvfs_setup() call zfs_unregister_callbacks().
Previously there was no synchronization between those calls and the calls
in the re-mounting case. That could lead to concurrent execution
and a crash.

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=180060
http://article.gmane.org/gmane.os.illumos.zfs/2833

Diff Detail

Repository
rS FreeBSD src repository
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

avg updated this revision to Diff 6320.Jun 19 2015, 7:30 AM
avg retitled this revision from to zfs_mount(MS_REMOUNT): protect zfs_(un)register_callbacks calls.
avg updated this object.
avg edited the test plan for this revision. (Show Details)
avg added a reviewer: mahrens.
avg added a subscriber: ZFS.
delphij added inline comments.Jun 19 2015, 8:04 AM
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
1256

I wonder if we should mark zfs_unregister_callbacks static, just like zfs_register_callbacks? (Or is this intentional to make them DTrace proble eligible?)

1934

I'm not very convinced that we can safely do unregister without having z_teardown_lock held here... If it's safe, maybe we should document it in the comment?

delphij added a reviewer: pho.Jun 19 2015, 8:04 AM
avg added inline comments.Jun 19 2015, 8:17 AM
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
1256

It looks like this function can be made static, but I'd do it as a separate commit.

1934

If !unmounting we, in fact, do hold the lock here.
And if unmounting then no other thread can operate on the filesystem because of z_unmounted.
See line 1886 above and 1918 too.

delphij accepted this revision.Jun 19 2015, 6:42 PM
delphij edited edge metadata.
This revision is now accepted and ready to land.Jun 19 2015, 6:42 PM
pho accepted this revision.Jun 20 2015, 7:34 AM
pho edited edge metadata.

No problems seen with the few zfs tests I have.

mahrens accepted this revision.Jun 20 2015, 3:30 PM
mahrens edited edge metadata.
will accepted this revision.Jun 23 2015, 3:59 PM
will edited edge metadata.

LGTM.

avg closed this revision.Jul 2 2015, 9:14 AM

Committed in rS285021