Skip to content
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

Suggestion for patch import from the Illumos project #340

Closed
mmatuska opened this issue Jul 26, 2011 · 4 comments
Closed

Suggestion for patch import from the Illumos project #340

mmatuska opened this issue Jul 26, 2011 · 4 comments
Milestone

Comments

@mmatuska
Copy link
Contributor

Hello, in FreeBSD we have imported the following from the Illumos project:

  1. Issue tar: a.out: file changed as we read it #764: panic in zfs:dbuf_sync_list
    https://www.illumos.org/issues/764
    illumos/illumos-gate@3f2366c
  2. Issue Load average inflated by +2 #175: zfs vdev cache consumes excessive memory
    https://www.illumos.org/issues/175
    illumos/illumos-gate@b68a40a
  3. Issue zfs mount -a fails #278: get rid of python dependencies
    https://www.illumos.org/issues/278
    illumos/illumos-gate@1af68be
  4. Issue Failed to load ZFS module stack #510: 'zfs get' enhancement - mountpoint as an argument
    https://www.illumos.org/issues/510
    illumos/illumos-gate@5ead3ed
  5. Issue raidz1: device IO failure when zfs filesystem is full #742: Resurrect the ZFS "aclmode" property
    https://www.illumos.org/issues/742
    illumos/illumos-gate@a3c49ce
  6. Issue ztest spa_vdev_add() returns EBUSY #1051: zfs should handle imbalanced luns
    https://www.illumos.org/issues/1051
    illumos/illumos-gate@09c9d37
  7. Issue pool import cause panic in arc_read_nolock / zil_parse #1092: zfs refratio property
    https://www.illumos.org/issues/1092
    illumos/illumos-gate@187d6ac
  8. Issue zvol swap devices #883: ZIL reuse during remount can lead to data corruption
    https://www.illumos.org/issues/883
    illumos/illumos-gate@c9ba2a4
  9. Issue Fix for scripts/common.sh.in not to return a non loop device.  #1043: Recursive destroy of zfs snapshot fails on non-existing target snapshot (my patch)
    https://www.illumos.org/issues/1043
    https://www.illumos.org/attachments/217/recursive_dataset_destroy.patch
@behlendorf
Copy link
Contributor

Thanks! I'll certainly get the bulk of these applied right away, a few others like the pythons dependencies I want to apply but that's a fair bit of work.

I'd like to more closely following both the FreeBSD and Illumos ZFS changes. Perhaps you can recommend the best way to keep up to date?

@mmatuska
Copy link
Contributor Author

I imported the Python dependency removal to our tree and it was easier than I expected. It was good for our tree because we don't have Python in our base system (we needed an extra port of the Python library). I mailed you some info about tracing latest changes in FreeBSD and Illumos.

@behlendorf
Copy link
Contributor

The Illumos branch contains all but one (alcmode) of the suggested patches which bringing us up to date with Illumos.

  • 'raidz1: device IO failure when zfs filesystem is full #742: Resurrect the ZFS "aclmode" property ' cannot be applied until we get ACLs fully integrated on Linux.
  • The missing Oracle 'zdb -vvv broken after zfs diff integration' patch was also applied.
  • While the python dependency patch was applied, and this is a BIG step in the right direction, this does not resolve the Linux integration issues with zfs delegations. This will have to be fixed in a follow up patch. It does however address the issues with 'zfs hold|holds|release'.

behlendorf pushed a commit that referenced this issue Aug 1, 2011
References to Illumos issue and patch:
- illumos/illumos-gate@163eb7ff

Signed-off-by: Brian Behlendorf <[email protected]>
Issue #340
behlendorf pushed a commit that referenced this issue Aug 1, 2011
Hypothesis about what's going on here.

At some time in the past, something, i.e. dnode_reallocate()
calls one of:
dbuf_rm_spill(dn, tx);

These will do:
dbuf_rm_spill(dnode_t *dn, dmu_tx_t *tx)
dbuf_free_range(dn, DMU_SPILL_BLKID, DMU_SPILL_BLKID, tx)
dbuf_undirty(db, tx)

Currently dbuf_undirty can leave a spill block in dn_dirty_records[],
(it having been put there previously by dbuf_dirty) and free it.
Sometime later, dbuf_sync_list trips over this reference to free'd
(and typically reused) memory.

Also, dbuf_undirty can call dnode_clear_range with a bogus
block ID. It needs to test for DMU_SPILL_BLKID, similar to
how dnode_clear_range is called in dbuf_dirty().

References to Illumos issue and patch:
- https://www.illumos.org/issues/764
- illumos/illumos-gate@3f2366c2bb

Reviewed by: George Wilson <[email protected]>
Reviewed by: [email protected]
Reviewed by: Albert Lee <[email protected]
Approved by: Garrett D'Amore <[email protected]>

Signed-off-by: Brian Behlendorf <[email protected]>
Issue #340
behlendorf pushed a commit that referenced this issue Aug 1, 2011
Note that with the current ZFS code, it turns out that the vdev
cache is not helpful, and in some cases actually harmful.  It
is better if we disable this.  Once some time has passed, we
should actually remove this to simplify the code.  For now we
just disable it by setting the zfs_vdev_cache_size to zero.
Note that Solaris 11 has made these same changes.

References to Illumos issue and patch:
- https://www.illumos.org/issues/175
- illumos/illumos-gate@b68a40a845

Reviewed by: George Wilson <[email protected]>
Reviewed by: Eric Schrock <[email protected]>
Approved by: Richard Lowe <[email protected]>

Signed-off-by: Brian Behlendorf <[email protected]>
Issue #340
behlendorf pushed a commit that referenced this issue Aug 1, 2011
The 'zfs get' command should be able to deal with mountpoint
as an argument.  It already works with 'zfs list' command:

  # zfs list /export/home/estibi
  NAME                       USED  AVAIL  REFER  MOUNTPOINT
  rpool/export/home/estibi  1.14G  3.86G  1.14G  /export/home/estibi

but it fails with 'zfs get':

  # zfs get all /export/home/estibi
  cannot open '/export/home/estibi': invalid dataset name

Reviewed by: Eric Schrock <[email protected]>
Reviewed by: Deano <[email protected]>
Reviewed by: Garrett D'Amore <[email protected]>
Approved by: Garrett D'Amore <[email protected]>

References to Illumos issue and patch:
- https://www.illumos.org/issues/510
- illumos/illumos-gate@5ead3ed965

Signed-off-by: Brian Behlendorf <[email protected]>
Issue #340
behlendorf pushed a commit that referenced this issue Aug 1, 2011
Today zfs tries to allocate blocks evenly across all devices.
This means when devices are imbalanced zfs will use lots of
CPU searching for space on devices which tend to be pretty
full.  It should instead fail quickly on the full LUNs and
move onto devices which have more availability.

Reviewed by: Eric Schrock <[email protected]>
Reviewed by: Matt Ahrens <[email protected]>
Reviewed by: Adam Leventhal <[email protected]>
Reviewed by: Albert Lee <[email protected]>
Reviewed by: Gordon Ross <[email protected]>
Approved by: Garrett D'Amore <[email protected]>

References to Illumos issue and patch:
- https://www.illumos.org/issues/510
- illumos/illumos-gate@5ead3ed965

Signed-off-by: Brian Behlendorf <[email protected]>
Issue #340
behlendorf pushed a commit that referenced this issue Aug 1, 2011
Add a "REFRATIO" property, which is the compression ratio based on
data referenced. For snapshots, this is the same as COMPRESSRATIO,
but for filesystems/volumes, the COMPRESSRATIO is based on the
data "USED" (ie, includes blocks in children, but not blocks
shared with the origin).

This is needed to figure out how much space a filesystem would
use if it were not compressed (ignoring snapshots).

Reviewed by: George Wilson <[email protected]>
Reviewed by: Adam Leventhal <[email protected]>
Reviewed by: Dan McDonald <[email protected]>
Reviewed by: Richard Elling <[email protected]>
Reviewed by: Mark Musante <[email protected]>
Reviewed by: Garrett D'Amore <[email protected]>
Approved by: Garrett D'Amore <[email protected]>

References to Illumos issue and patch:
- https://www.illumos.org/issues/1092
- illumos/illumos-gate@187d6ac08a

Signed-off-by: Brian Behlendorf <[email protected]>
Issue #340
behlendorf pushed a commit that referenced this issue Aug 1, 2011
Moving the zil_free() cleanup to zil_close() prevents this
problem from occurring in the first place.  There is a very
good description of the issue and fix in Illumus #883.

Reviewed by: Matt Ahrens <[email protected]>
Reviewed by: Adam Leventhal <[email protected]>
Reviewed by: Albert Lee <[email protected]>
Reviewed by: Gordon Ross <[email protected]>
Reviewed by: Garrett D'Amore <[email protected]>
Reivewed by: Dan McDonald <[email protected]>
Approved by: Gordon Ross <[email protected]>

References to Illumos issue and patch:
- https://www.illumos.org/issues/883
- illumos/illumos-gate@c9ba2a43cb

Signed-off-by: Brian Behlendorf <[email protected]>
Issue #340
behlendorf pushed a commit that referenced this issue Aug 1, 2011
Prior to revision 11314 if a user was recursively destroying
snapshots of a dataset the target dataset was not required to
exist.  The zfs_secpolicy_destroy_snaps() function introduced
the security check on the target dataset, so since then if the
target dataset does not exist, the recursive destroy is not
performed.  Before 11314, only a delete permission check on
the snapshot's master dataset was performed.

Steps to reproduce:
zfs create pool/a
zfs snapshot pool/a@s1
zfs destroy -r pool@s1

Therefore I suggest to fallback to the old security check, if
the target snapshot does not exist and continue with the destroy.

References to Illumos issue and patch:
- https://www.illumos.org/issues/1043
- https://www.illumos.org/attachments/217/recursive_dataset_destroy.patch

Signed-off-by: Brian Behlendorf <[email protected]>
Issue #340
behlendorf pushed a commit that referenced this issue Aug 1, 2011
Remove all python and pyzfs dependencies for consistency and
to ensure full functionality even in a mimimalist environment.

Reviewed by: [email protected]
Reviewed by: [email protected]
Reviewed by: [email protected]
Reviewed by: [email protected]
Approved by: [email protected]

References to Illumos issue and patch:
- https://www.illumos.org/issues/278
- illumos/illumos-gate@1af68beac3

Signed-off-by: Brian Behlendorf <[email protected]>
Issue #340
Issue #160

Signed-off-by: Brian Behlendorf <[email protected]>
@behlendorf
Copy link
Contributor

Thanks again for the nice list of updates. Everything has been reviewed, updated as needed, and merged in to master. The only two exceptions are the following patches which still need some work to be done.

Issue #170 - Illumos 742: Resurrect the ZFS "aclmode" property
Issue #160 - Illumos 278: get rid zfs of python and pyzfs dependencies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants