-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Newer Illumos code #1775
Newer Illumos code #1775
Conversation
As an additional note, I seem to have gotten the userland tools building with C99, but as illumos 4047 demonstrated, the kernel modules are still being built as C89. |
I did a rebase to fix a mistake where I gave myself authorship on one of the illumos commits... now the history being shown by github is incorrect (although my own local history seems right). All of my commits are supposed to precede the Illumos commits. |
@ryao This is great! I'll make the time to get you feedback on this, I'd love it if the 0.6.3 caught us largely up with Illumos. |
@dweeezil Since you're familiar with this issues involved in bring in changes from Illumos it would be great if you could also review these patches. |
I have refreshed the pull request with the following changes:
With that said, someone still needs to go through and revert modifications that occurred because ZFSOnLinux only called the constructors/destructors once per SLAB. @nedbass' work to port 4045 does this and a few other places seem to do it too. I have not touched this yet, but the corresponding SPL pull request includes the modifications required to support this. |
A minor refresh. I just did a review of the output of
Also, I found that my above list is missing the following two commits:
These might fall under the "do not make sense on Linux" category given that we don't have ACL support working yet. |
This lock unnecessarily serializes an important path. The fast write variables are updated using atomic operations, which need no protection. The code snippet being protected is already protected by the spa_lock. We remove mc_fastwrite_lock and add an assertion to verify that we have a spa_lock. This permits multiple threads to run through the metaslab_alloc_dva() function in parallel as they do on other implementations. Signed-off-by: Richard Yao <ryao@gentoo.org>
@behlendorf I took a cursory glance at this wad as soon as it was posted and was planning on giving it a more thorough look this weekend. |
I've still not had much time to look at this yet (it's huge!) but after fiddling with it a bit, I came up with a couple of patches in dweeezil/zfs@1a91683; one to deal with a nvlist allocation and the other to deal with a duplicate preprocessor definition. I've been simultaneously looking the code and trying to run it. Another observation is that I can't unload the zfs.ko module (if anything nontrivial has been done subsequent to its loading) without reverting the recent constructor/destructor patch to spl. @ryao, is this to be expected at this point? The rmmod process blocks in D state but I've not had a chance to figure out why or where it's blocking. FInally, I'll note that trying to do a Hopefully I'll have more time to look at this throughout the coming week. |
@dweeezil You seem to have found a couple of regressions. I am not sure what caused the module unloading issue, but the |
@ryao I suspect you'll want to remove the extern declaration of zfs_txg_synctime_ms in dsl_pool.h as part of the 4045 patch. |
@kpande Did you also adjust the length parameter? See dweeezil/zfs@29b2094. |
@kpande Try dweeezil/zfs@fdffb9e. The failure condition you discovered was a result of the illumos 3744 patch. |
@kpande that would be a different bug. Thanks for letting us know about it. |
I have pushed a refresh. Here are the changes.
I spent the better part of the past week trying to get a new workstation working (sadly, the hardware is not cooperating), so there is not as much here as I would have liked to have. Here is what is still left to do:
|
This lock unnecessarily serializes an important path. The fast write variables are updated using atomic operations, which need no protection. The code snippet being protected is already protected by the spa_lock. We remove mc_fastwrite_lock and add an assertion to verify that we have a spa_lock. This permits multiple threads to run through the metaslab_alloc_dva() function in parallel as they do on other implementations. Signed-off-by: Richard Yao <ryao@gentoo.org>
This was accidentally removed by overzealous commenting. Signed-off-by: Richard Yao <ryao@gentoo.org>
Modifying the length of a string returned by strdup() is incorrect because strfree() is allowed to use strlen() to determine which slab cache was used to do the allocation. Signed-off-by: Richard Yao <ryao@gentoo.org>
The resolution of a merge conflict when merging Illumos openzfs#3464 caused us to invert the order couple of function calls in zio_free_sync() versus what they are in Illumos. Signed-off-by: Richard Yao <ryao@gentoo.org>
This resolves merge conflicts when merging Illumos openzfs#3588 and Illumos openzfs#4047. Signed-off-by: Richard Yao <ryao@gentoo.org>
Upstream Illumos uses C99, but we have been building as C89. This required backporting patches from Illumos from C99 to C89. There are no known ABI compatibility issues when mixing C89 and C99 code. so lets just build as C99 and save ourselves some trouble. This patch modifies the autotools checks, userland code and kernel code to use C89. Note that the Linux kernel passes -Wdeclaration-after-statement, so we must pass -Wno-declaration-after-statement to counteract that. We also do a little cleanup to simplify the makefiles. Signed-off-by: Richard Yao <ryao@gentoo.org>
References: illumos/illumos-gate@d39ee14 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775 Porting notes: 1. This commit was so old that only two lines applied to the modern code base.
References: illumos/illumos-gate@44bffe0 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3582 zfs_delay() should support a variable resolution 3584 DTrace sdt probes for ZFS txg states Reviewed by: Matthew Ahrens <mahrens@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Reviewed by: Christopher Siden <christopher.siden@delphix.com> Reviewed by: Dan McDonald <danmcd@nexenta.com> Reviewed by: Richard Elling <richard.elling@dey-sys.com> Approved by: Garrett D'Amore <garrett@damore.org> References: https://www.illumos.org/issues/3582 illumos/illumos-gate@0689f76 Ported by: Ned Bass <bass6@llnl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3699 zfs hold or release of a non-existent snapshot does not output error 3739 cannot set zfs quota or reservation on pool version < 22 Reviewed by: Matthew Ahrens <mahrens@delphix.com> Reviewed by: Eric Shrock <eric.schrock@delphix.com> Approved by: Dan McDonald <danmcd@nexenta.com> References: https://www.illumos.org/issues/3699 https://www.illumos.org/issues/3739 illumos/illumos-gate@013023d Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3741 zfs needs better comments Reviewed by: Matthew Ahrens <mahrens@delphix.com> Reviewed by: Eric Schrock <eric.schrock@delphix.com> Approved by: Christopher Siden <christopher.siden@delphix.com> References: https://www.illumos.org/issues/3741 illumos/illumos-gate@3e30c24 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3742 zfs comments need cleaner, more consistent style Reviewed by: Matthew Ahrens <mahrens@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Reviewed by: Eric Schrock <eric.schrock@delphix.com> Approved by: Christopher Siden <christopher.siden@delphix.com> References: https://www.illumos.org/issues/3742 illumos/illumos-gate@f717074 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775 Porting notes: 1. The change to zfs_vfsops.c was dropped because it involves zfs_mount_label_policy, which does not exist in the Linux port.
3743 zfs needs a refcount audit Reviewed by: Matthew Ahrens <mahrens@delphix.com> Reviewed by: Eric Schrock <eric.schrock@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Approved by: Christopher Siden <christopher.siden@delphix.com> References: https://www.illumos.org/issues/3743 illumos/illumos-gate@b287be1 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3744 zfs shouldn't ignore errors unmounting snapshots Reviewed by: Matthew Ahrens <mahrens@delphix.com> Approved by: Christopher Siden <christopher.siden@delphix.com> References: https://www.illumos.org/issues/3744 illumos/illumos-gate@fc7a6e3 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775 Porting notes: 1. There is no clear way to distinguish between a failure when we tried to unmount the snapdir of a zvol (which does not exist) and the failure when we try to unmount a snapdir of a dataset, so the changes to zfs_unmount_snap() were dropped in favor of an altered Linux function that unconditionally returns 0.
3745 zpool create should treat -O mountpoint and -m the same 3811 zpool create -o altroot=/xyz -O mountpoint=/mnt ignores the mountpoint option Reviewed by: Matthew Ahrens <mahrens@delphix.com> Approved by: Christopher Siden <christopher.siden@delphix.com> References: https://www.illumos.org/issues/3745 https://www.illumos.org/issues/3811 illumos/illumos-gate@8b71377 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3740 Poor ZFS send / receive performance due to snapshot hold / release processing Reviewed by: Matthew Ahrens <mahrens@delphix.com> Approved by: Christopher Siden <christopher.siden@delphix.com> References: https://www.illumos.org/issues/3740 illumos/illumos-gate@a7a845e Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775 Porting notes: 1. 13fe019 introduced a merge conflict in dsl_dataset_user_release_tmp where some variables were moved outside of the preprocessor directive. 2. dea9dfe made the previous merge conflict worse by switching KM_SLEEP to KM_PUSHPAGE. This is notable because this commit refactors the code, adding a new KM_SLEEP allocation. It is not clear to me whether this should be converted to KM_PUSHPAGE. 3. We had a merge conflict in libzfs_sendrecv.c because of copyright notices. 4. Several small C99 compatibility fixed were made.
3818 zpool status -x should report pools with removed l2arc devices Reviewed by: Saso Kiselkov <skiselkov.ml@gmail.com> Reviewed by: George Wilson <gwilson@zfsmail.com> Approved by: Christopher Siden <christopher.siden@delphix.com> References: https://www.illumos.org/issues/3818 illumos/illumos-gate@7f2416e Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3829 fix for 3740 changed behavior of zfs destroy/hold/release ioctl Reviewed by: Matt Amdur <matt.amdur@delphix.com> Reviewed by: Christopher Siden <christopher.siden@delphix.com> Approved by: Richard Lowe <richlowe@richlowe.net> References: https://www.illumos.org/issues/3829 illumos/illumos-gate@bb6e707 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3894 zfs should not allow snapshot of inconsistent dataset Reviewed by: Matthew Ahrens <mahrens@delphix.com> Approved by: Gordon Ross <gwr@nexenta.com> References: https://www.illumos.org/issues/3894 illumos/illumos-gate@ca48f36 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3888 zfs recv -F should destroy any snapshots created since the incremental source Reviewed by: George Wilson <george.wilson@delphix.com> Reviewed by: Adam Leventhal <ahl@delphix.com> Reviewed by: Peng Dai <peng.dai@delphix.com> Approved by: Richard Lowe <richlowe@richlowe.net> References: https://www.illumos.org/issues/3888 illumos/illumos-gate@34f2f8c Porting notes: 1. Commit 1fde1e3 wrapped a declaration in dsl_dataset_modified_since_lastsnap in ASSERTV(). The ASSERTV() and local variable have been removed to avoid an unused variable warning. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Ported-by: Richard Yao <ryao@gentoo.org> Issue openzfs#1775
3875 panic in zfs_root() after failed rollback Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com> Reviewed by: Matthew Ahrens <mahrens@delphix.com> Approved by: Gordon Ross <gwr@nexenta.com> References: https://www.illumos.org/issues/3875 illumos/illumos-gate@91948b5 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3836 zio_free() can be processed immediately in the common case Reviewed by: George Wilson <george.wilson@delphix.com> Reviewed by: Adam Leventhal <ahl@delphix.com> Approved by: Dan McDonald <danmcd@nexenta.com> References: https://www.illumos.org/issues/3836 illumos/illumos-gate@9cb154a Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3834 incremental replication of 'holey' file systems is slow Reviewed by: Adam Leventhal <ahl@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Approved by: Richard Lowe <richlowe@richlowe.net> References: https://www.illumos.org/issues/3834 illumos/illumos-gate@ca48f36 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3909 "zfs send -D" does not work Reviewed by: Matthew Ahrens <mahrens@delphix.com> Approved by: Christopher Siden <christopher.siden@delphix.com> References: https://www.illumos.org/issues/3909 illumos/illumos-gate@36f7455 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3973 zfs_ioc_rename alters passed in zc->zc_name Reviewed by: Matthew Ahrens <mahrens@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Approved by: Christopher Siden <christopher.siden@delphix.com> References: https://www.illumos.org/issues/3973 illumos/illumos-gate@a0c1127 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3955 ztest failure: assertion refcount_count(&tx->tx_space_written) + delta <= tx->tx_space_towrite Reviewed by: Adam Leventhal <ahl@delphix.com> Reviewed by: Dan Kimmel <dan.kimmel@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Approved by: Richard Lowe <richlowe@richlowe.net> References: https://www.illumos.org/issues/3955 illumos/illumos-gate@be9000c Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3949 ztest fault injection should avoid resilvering devices 3950 ztest: deadman fires when we're doing a scan 3951 ztest hang when running dedup test 3952 ztest: ztest_reguid test and ztest_fault_inject don't place nice together Reviewed by: Matthew Ahrens <mahrens@delphix.com> Reviewed by: Adam Leventhal <ahl@delphix.com> Approved by: Richard Lowe <richlowe@richlowe.net> References: https://www.illumos.org/issues/3949 https://www.illumos.org/issues/3950 https://www.illumos.org/issues/3951 https://www.illumos.org/issues/3952 illumos/illumos-gate@2c1e2b4 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775 Porting notes: 1. The deadman thread was removed from ztest during the original port because it depended on Solaris thr_create() interface. This functionality should be reintroduced using the more portable pthreads.
3996 want a libzfs_core API to rollback to latest snapshot Reviewed by: Christopher Siden <christopher.siden@delphix.com> Reviewed by: Adam Leventhal <ahl@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Reviewed by: Andy Stormont <andyjstormont@gmail.com> Approved by: Richard Lowe <richlowe@richlowe.net> References: https://www.illumos.org/issues/3996 illumos/illumos-gate@a7027df Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
4047 panic from dbuf_free_range() from dmu_free_object() while doing zfs receive Reviewed by: Adam Leventhal <ahl@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Approved by: Dan McDonald <danmcd@nexenta.com> References: https://www.illumos.org/issues/4047 illumos/illumos-gate@713d6c2 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775 Porting notes: 1. The exported symbol dmu_free_object() was renamed to dmu_free_long_object() in Illumos.
4061 libzfs: memory leak in iter_dependents_cb() Reviewed by: Jeffry Molanus <jeffry.molanus@nexenta.com> Reviewed by: Boris Protopopov <boris.protopopov@nexenta.com> Reviewed by: Andy Stormont <andyjstormont@gmail.com> Reviewed by: Matthew Ahrens <mahrens@delphix.com> Approved by: Dan McDonald <danmcd@nexenta.com> References: https://www.illumos.org/issues/4061 illumos/illumos-gate@2fbdf8d Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
4046 dsl_dataset_t ds_dir->dd_lock is highly contended Reviewed by: Eric Schrock <eric.schrock@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Approved by: Garrett D'Amore <garrett@damore.org> References: https://www.illumos.org/issues/4046 illumos/illumos-gate@b62969f Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775 Porting notes: 1. This commit removed dsl_dataset_namelen in Illumos, but that appears to have been removed from ZFSOnLinux in an earlier commit.
3954 metaslabs continue to load even after hitting zfs_mg_alloc_failure limit 4080 zpool clear fails to clear pool 4081 need zfs_mg_noalloc_threshold Reviewed by: Adam Leventhal <ahl@delphix.com> Reviewed by: Matthew Ahrens <mahrens@delphix.com> Approved by: Richard Lowe <richlowe@richlowe.net> References: https://www.illumos.org/issues/3954 https://www.illumos.org/issues/4080 https://www.illumos.org/issues/4081 illumos/illumos-gate@22e3098 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
4082 zfs receive gets EFBIG from dmu_tx_hold_free() Reviewed by: Eric Schrock <eric.schrock@delphix.com> Reviewed by: Christopher Siden <christopher.siden@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Approved by: Richard Lowe <richlowe@richlowe.net> References: https://www.illumos.org/issues/4082 illumos/illumos-gate@5253393 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
4168 ztest assertion failure in dbuf_undirty 4169 verbatim import causes zdb to segfault 4170 zhack leaves pool in ACTIVE state Reviewed by: Adam Leventhal <ahl@delphix.com> Reviewed by: Eric Schrock <eric.schrock@delphix.com> Reviewed by: Matthew Ahrens <mahrens@delphix.com> Approved by: Dan McDonald <danmcd@nexenta.com> References: https://www.illumos.org/issues/4168 https://www.illumos.org/issues/4169 https://www.illumos.org/issues/4170 illumos/illumos-gate@7fdd916 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
3995 Memory leak of compressed buffers in l2arc_write_done References: https://illumos.org/issues/3995 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#1688 Issue openzfs#1775
A couple of kmem_alloc() allocations were using KM_SLEEP in the sync thread context. These were accidentally introduced by the recent set of Illumos patches. The solution is to switch to KM_PUSHPAGE. dsl_dataset_promote_sync() -> promote_hold() -> snaplist_make() -> kmem_alloc(sizeof (*snap), KM_SLEEP); dsl_dataset_user_hold_sync() -> dsl_onexit_hold_cleanup() -> kmem_alloc(sizeof (*ca), KM_SLEEP) Signed-off-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#1775
This depends on openzfs/spl#300. It supersedes #1696, #1760 and #1772.
It includes some fixes that I wrote when porting the changes. I split them out and placed them before the Illumos commits to make backporting them easier. This is not quite ready to merge yet, but it is ready for comments.
I ran
git log usr/src/uts/common/fs/zfs
and reviewed all commits made since the start of 2012 to obtain a list of commits that we were missing. In addition, I usedgit blame
to identify the causes of merge conflicts, which identified some even older commits that we were missing. The following commits have been omitted for any of the following reasons: