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

ZFS-Crypto is behind ZoL #40

Open
FransUrbo opened this issue Dec 6, 2013 · 5 comments
Open

ZFS-Crypto is behind ZoL #40

FransUrbo opened this issue Dec 6, 2013 · 5 comments

Comments

@FransUrbo
Copy link
Contributor

Sorry for adding another one of these - just a heads up. The commit was done just a few minutes ago, but I'm having a problem with my pool/volumes/something and I'm pulling a couple times a day hoping something will fix it...

It's another Illumos commit - 'Illumos #4045 write throttle & i/o scheduler performance work' (openzfs/zfs@e8b96c6).

I'm trying to merge it myself, but there's so many things I don't understand about the inner workings of ZFS...

@FransUrbo
Copy link
Contributor Author

Ok, looking closer at this, it seems most of the stuff was changes of number of arguments to zio_rewrite(). And two other things, that I don't know what they're for but I just deleted them. Looked harmless enough :)

diff --git a/module/zfs/zil.c b/module/zfs/zil.c
index 3c7b97e..374a91d 100644
--- a/module/zfs/zil.c
+++ b/module/zfs/zil.c
@@ -960,10 +960,6 @@ zil_lwb_write_init(zilog_t *zilog, lwb_t *lwb)
        if (rzio == NULL) {
                zilog->zl_root_zio = rzio = zio_root(zilog->zl_spa, NULL, NULL,
                    ZIO_FLAG_CANFAIL);
-               if (zilog->zl_logbias == ZFS_LOGBIAS_LATENCY)
-                       rzio->io_priority = ZIO_PRIORITY_LOG_WRITE;
-               else
-                       rzio->io_priority = ZIO_PRIORITY_ASYNC_WRITE;
        }

        /* Lock so zil_sync() doesn't fastwrite_unmark after zio is created */

and

diff --git a/module/zfs/zio.c b/module/zfs/zio.c
index c95e2c2..7da0279 100644
--- a/module/zfs/zio.c
+++ b/module/zfs/zio.c
@@ -799,9 +799,6 @@ zio_rewrite(zio_t *pio, spa_t *spa, uint64_t txg, blkptr_t *bp, void *data,
            ZIO_TYPE_WRITE, priority, flags, NULL, 0, zb,
            ZIO_STAGE_OPEN, ZIO_REWRITE_PIPELINE);

-       if (zp != NULL)
-               zio->io_prop = *zp;
-
        return (zio);
 }

@FransUrbo
Copy link
Contributor Author

It at least compiles without problem (currently building packages and will install and test them within the hour).

How should I proceed? A pull request will be quite big (include A LOT of ZoL stuff not included - yet - in the ZFS-Crypto/0.6.3 branch).

@sempervictus
Copy link

ABD patch set is making the merge rather problematic. The hashing algo's have changed, as have the calls to them. Looks like sha-mac wasnt updated as ZoL doesnt use it.
There's also a PR up for optmizing the SHA256 routines with ASM for x86-64, may be an entry point to doing the same for crypto.

@lundman
Copy link
Contributor

lundman commented Jun 4, 2014

The SPL code for SHA uses Linux crypto, so should already be assembler, and AESNI support. ZOL doesn't use the Linux crypto API, which is probably why they are looking at improving it. Be interesting to change ZOL to use the SPL call instead and see how it compares :)

Let's ask rogue to do another merge soon and see how bad it is. 0.6.3 can't be far off now?

@zfsrogue
Copy link
Owner

merge finished again, plz test.

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

4 participants