Proposed patches to stable 0.7 branch #6752
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commit d3c2ae1 introduced a dbuf cache with a default size of the minimum of 100M or 1/32 maximum ARC size. (These figures may be adjusted using dbuf_cache_max_bytes and dbuf_cache_max_shift.) The dbuf cache is counted as metadata for the purposes of ARC size calculations. On a 1GB box the ARC maximum size defaults to c_max 493M which gives a dbuf cache default minimum size of 15.4M, and the ARC metadata defaults to minimum 16M. I.e. the dbuf cache is an significant proportion of the minimum metadata size. With other overheads involved this actually means the ARC metadata doesn't get down to the minimum. This patch dynamically scales the dbuf cache to the target ARC size instead of statically scaling it to the maximum ARC size. (The scale is still set by dbuf_cache_max_shift and the maximum size is still fixed by dbuf_cache_max_bytes.) Using the target ARC size rather than the current ARC size is done to help the ARC reach the target rather than simply focusing on the current size. Reviewed-by: Chunwei Chen <tuxoko@gmail.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Chris Dunlop <chris@onthe.net.au> Issue openzfs#6506 Closes openzfs#6561
When receiving a FREEOBJECTS record, receive_freeobjects() incorrectly skips a freed object in some cases. Specifically, this happens when the first object in the range to be freed doesn't exist, but the second object does. This leaves an object allocated on disk on the receiving side which is unallocated on the sending side, which may cause receiving subsequent incremental streams to fail. The bug was caused by an incorrect increment of the object index variable when current object being freed doesn't exist. The increment is incorrect because incrementing the object index is handled by a call to dmu_object_next() in the increment portion of the for loop statement. Add test case that exposes this bug. Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ned Bass <bass6@llnl.gov> Closes openzfs#6694 Closes openzfs#6695
The vdev_copy_uberblocks() function should use abd_alloc_linear() to allocate ub_abd, because abd_to_buf(ub_abd)) is used later. Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Isaac Huang <he.huang@intel.com> Closes openzfs#6718 Closes openzfs#6713
Currently `if` statement includes an assignment (from a function return value) and a equality check. The parenthesis are in the incorrect place, currently the code clobbers the function return value because of this. We can fix this by simplifying the `if` statement. `if (foo != 0)` can be more succinctly expressed as `if (foo)` Remove the equality check, add parenthesis to correct the statement. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Chris Dunlop <chris@onthe.net.au> Signed-off-by: Tobin C. Harding <me@tobin.cc> Closes openzfs#6685 Close openzfs#6719
Make two instances of the same change. Change bitwise AND (&) to logical AND (&&). Currently the code uses a bitwise AND between two boolean values. In the first instance; The first operand is a flag that has been bitwise combined with a bit mask to get a boolean value as to whether a file has group write permissions set. The second operand used is a struct member that is intended as a boolean flag not a bit mask. In the second instance the argument is the same except with world write permissions instead of group write (S_IWOTH, S_IWGRP). Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Chris Dunlop <chris@onthe.net.au> Signed-off-by: Tobin C. Harding <me@tobin.cc> Closes openzfs#6684 Closes openzfs#6722
On Void Linux (x86_64 musl) libgcc_s.so is located in "/usr/lib" so it is not found by dracut and it produces an error. Add a simple additional path check for "/usr/lib/libgcc_s.so*" and install it in the initramfs. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: privb0x23 <privb0x23@users.noreply.github.com> Closes openzfs#6715
With the addition of the ABD changes consumption of the virtual address space has been greatly reduced. This exposed an issue on CONFIG_HIGHMEM systems where free memory was being calculated incorrectly. Functionally this didn't cause any major problems prior to ABD because a lack of available virtual address space was used as an indicator of low memory. This patch makes the following changes to address the issue and in the process realigns the code further with OpenZFS. There are no substantive changes in behavior for 64-bit systems. * Added CONFIG_HIGHMEM case to the arc_all_memory() and arc_free_memory() functions to only consider low memory pages on CONFIG_HIGHMEM systems. * The arc_free_memory() function was updated to return bytes instead of pages to be consistent with the other helper functions. In user space we make up some reasonable values since currently only testing is performed in this context. * Adds three new values to the arcstats kstat to provide visibility in to the ARC's assessment of the memory situation: memory_all_bytes, memory_free_bytes, and memory_available_bytes. * Added kmem_reap() call to arc_available_memory() for 32-bit builds to realign code with OpenZFS. * Reduced size of test file in /async_destroy_001_pos.ksh to speed up test case. Multiple txgs are still required. * Move vdevs used by zpool_clear_001_pos and zpool_upgrade_002_pos to TEST_BASE_DIR location to speed up test cases. Reviewed-by: David Quigley <david.quigley@intel.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#5352 Closes openzfs#6734
loli10K
reviewed
Oct 12, 2017
| test_fs_setup $sendfs $recvfs | ||
| log_must zfs unmount $sendfs | ||
| test_fs_setup $sendfs $recvfs $streamfs | ||
| log_must zfs unmount -f $sendfs |
There was a problem hiding this comment.
Sharp eye. I'll remove that.
|
@Fabian-Gruenbichler I forgot to mention that your commit 48fbb9d touched I only superficially understand 48fbb9d, so I'm not 100% sure it makes sense to just ignore the change made to |
Because resuming from a token requires "guid" -> "snapshot" mapping we have to walk the whole dataset hierarchy to find the right snapshot to send; when both source and destination exists, for an incremental resumable stream, libzfs gets confused and picks up the wrong snapshot to send from: this results in attempting to send "destination@snap1 -> source@snap2" instead of "source@snap1 -> source@snap2" which fails with a "Invalid cross-device link" error (EXDEV). Fix this by adjusting the logic behind dataset traversal in zfs_iter_children() to pick the right snapshot to send from. Additionally update dry-run 'zfs send -t' to print its output to stderr: this is consistent with other dry-run commands. Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: loli10K <ezomori.nozomu@gmail.com> Closes openzfs#6618 Closes openzfs#6619 Closes openzfs#6623
All objects after the last written or freed object are not supposed to exist after receiving the stream. Free them accordingly, as if a freeobjects record for them had been included in the stream. Reviewed by: Paul Dagnelie <pcd@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> Closes openzfs#5699 Closes openzfs#6507 Closes openzfs#6616
When sending an incremental stream based on a snapshot, the receiving side must have the same base snapshot. Thus we do not need to send FREEOBJECTS records for any objects past the maximum one which exists locally. This allows us to send incremental streams (again) to older ZFS implementations (e.g. ZoL < 0.7) which actually try to free all objects in a FREEOBJECTS record, instead of bailing out early. Reviewed by: Paul Dagnelie <pcd@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> Closes openzfs#5699 Closes openzfs#6507 Closes openzfs#6616
* Correct ZFS snapshot listing * Disable "lvm is not available" message on quiet boot Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Alar Aun <spamtoaun@gmail.com> Closes openzfs#6700 Closes openzfs#6747
|
On Thu, Oct 12, 2017 at 02:59:12AM -0700, Antonio Russo wrote:
@Fabian-Gruenbichler I forgot to mention that your commit ***@***.*** touched `receive_object_range` in `dmu_send.c`, which, from my very cursory inspection of its history, was added to support encryption, and won't be included in this stable branch.
I only superficially understand ***@***.***, so I'm not 100% sure it makes sense to just ignore the change made to `receive_object_range`. Could you please double check me here?
yes, dropping that hunk is the correct way to go for backporting to 0.7.
|
aerusso
force-pushed
the
zfs-0.7-proposed
branch
from
c0249dd
to
7104163
Compare
Automatic dependency resolution is unreliable on many systems. Follow suit with existing code, and explicitly include icp in module dependencies. Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Antonio Russo <antonio.e.russo@gmail.com> Closes openzfs#6751
The current code base almost compiles on SPARC, but a few fixes are required for the code to compile (and work efficiently). Code in this PR comes from OpenZFS project which was initially dropped when porting the crypto framework. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Pengcheng Xu <i@jsteward.moe> Closes openzfs#6733 Closes openzfs#6738 Closes openzfs#6750
aerusso
force-pushed
the
zfs-0.7-proposed
branch
from
c84d4e0
to
89253be
Compare
|
Per the updates to PR 0.7.3, I've included e0922b0 and 085b501. This should also be pulling the correct SPL now, per your comment, @behlendorf. |
|
@aerusso thanks for doing the legwork on this. I'll look into pulling it in once buildbot finishes. |
|
@aerusso everything here looks fine. I just merged in your 0.7.3 SPL commit, so you can remove "Minimal requirement for SPL build compatibility". After that I can merge in this PR. |
aerusso
force-pushed
the
zfs-0.7-proposed
branch
from
89253be
to
485e6e2
Compare
|
I ended up just cherry picking these and pushed them to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
The PR needed section on 0.7.3 projects page listed some patches. I've attempted to resolve conflicts and get them able to merge onto the stable branch.
I'm half doing this for myself. I figured putting it here might avoid any duplicated effort, and centralize a discussion for the patches to the stable branch. If this is a bad idea, or a waste of effort, let me know.
Description
I've rebased the patches in 0.7.3 project onto the current stable head. In particular aee1dd4 touches code that has changed somewhat. @loli10K could you please make sure I've not made a terrible mistake here?
Motivation and Context
Some of the bugs addressed in these patches sound really annoying, and I'd like to avoid them if possible.
How Has This Been Tested?
I've built this, and installed it on a machine. I would like to use the build infrastructure here for more tests. I'll be putting it on more machines gradually.
Types of changes
Checklist:
Signed-off-by.