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

LZ4 compression #1217

Closed
behlendorf opened this issue Jan 17, 2013 · 9 comments
Closed

LZ4 compression #1217

behlendorf opened this issue Jan 17, 2013 · 9 comments
Labels
Type: Feature Feature request or new feature
Milestone

Comments

@behlendorf
Copy link
Contributor

Add the LZ4 compression algorithm which was recently merged in to Illumos, illumos/illumos-gate@a6f561b

@edillmann
Copy link
Contributor

I will take a look to get this working

@edillmann
Copy link
Contributor

Hi,

@behlendorf :Could you take a look to this commit :edillmann@6e744ad

This code "work's for me".

I didn't try to use slab's in this because i don't know where i should add the pool allocation and freeing code.

Regards,
Eric

@behlendorf
Copy link
Contributor Author

@edillmann It looks good, but let's get the code for the kmem cache added and then I'll do some more careful review and testing. My suggestion would be to initialize the kmem cache through zio_init() which already takes care of various other zio related caches. Something like this:

module/zfs/zio.c

void
zio_init(void) {
        ...
        lz4_init()
        zio_inject_init();
}

void
zio_fini(void) {
        ...
        lz4_fini();
        zio_inject_fini();
}

module/zfs/lz4.c

static kmem_cache_t *lz4_cache;

void
lz4_init(void)
{
        lz4_cache = kmem_cache_create("lz4_cache",
            24 * 1024, 0, NULL, NULL, NULL, NULL, NULL, 0);
}

void
lz4_fini(void)
{
        kmem_cache_destroy(lz4_cache);
}

@edillmann
Copy link
Contributor

Et voila : edillmann@cda42be

@behlendorf : Could you take a look at this commit.

I tried to follow illumos as much as possible. But removed dead (unused) code.

@behlendorf
Copy link
Contributor Author

@edillmann Comments inline, it largely looks good! If you can fix the lingering issues and open a pull request I'll give it another look.

@csiden
Copy link

csiden commented Jan 25, 2013

I tried to follow illumos as much as possible. But removed dead (unused) code.

@edillmann Do we have dead code upstream we should be removing or is it dead code due to linux specific changes?

@behlendorf
Copy link
Contributor Author

@csiden This version drops the following unused functions which do appear in the Illumos upstream version.

static int real_LZ4_uncompress(const char *source, char *dest, int osize);
static int LZ4_compressBound(int isize);

I'd also suggest dropping the _MSC_VER blocks unless you plan on building Illumos with Visual Studio...

@csiden
Copy link

csiden commented Jan 25, 2013

Thanks. I checked with Sašo who says he left those in intentionally to make it easier to to apply future patches from upstream LZ4 development. So, I probably won't go out of my way to remove the dead code from illumos until we see how frequent these updates are.

@behlendorf
Copy link
Contributor Author

9759c60 Illumos #3035 LZ4 compression support in ZFS and GRUB

unya pushed a commit to unya/zfs that referenced this issue Dec 13, 2013
3035 LZ4 compression support in ZFS and GRUB

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Christopher Siden <christopher.siden@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Christopher Siden <csiden@delphix.com>

References:
  illumos/illumos-gate@a6f561b
  https://www.illumos.org/issues/3035
  http://wiki.illumos.org/display/illumos/LZ4+Compression+In+ZFS

This patch has been slightly modified from the upstream Illumos
version to be compatible with Linux.  Due to the very limited
stack space in the kernel a lz4 workspace kmem cache is used.
Since we are using gcc we are also able to take advantage of the
gcc optimized __builtin_ctz functions.

Support for GRUB has been dropped from this patch.  That code
is available but those changes will need to made to the upstream
GRUB package.

Lastly, several hunks of dead code were dropped for clarity.  They
include the functions real_LZ4_uncompress(), LZ4_compressBound()
and the Visual Studio specific hunks wrapped in _MSC_VER.

Ported-by: Eric Dillmann <eric@jave.fr>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#1217
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Feature request or new feature
Projects
None yet
Development

No branches or pull requests

3 participants