Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1391563 > unrolled thread

Re: [PATCH v2] pstore: add lzo/lz4 compression support

Started byGeliang Tang <geliangtang@163.com>
First post2016-04-30 04:30 +0200
Last post2016-05-02 20:50 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2] pstore: add lzo/lz4 compression support Geliang Tang <geliangtang@163.com> - 2016-04-30 04:30 +0200
    Re: [PATCH v2] pstore: add lzo/lz4 compression support Kees Cook <keescook@chromium.org> - 2016-05-02 20:40 +0200
      RE: [PATCH v2] pstore: add lzo/lz4 compression support "Luck, Tony" <tony.luck@intel.com> - 2016-05-02 20:50 +0200

#1391563 — Re: [PATCH v2] pstore: add lzo/lz4 compression support

FromGeliang Tang <geliangtang@163.com>
Date2016-04-30 04:30 +0200
SubjectRe: [PATCH v2] pstore: add lzo/lz4 compression support
Message-ID<rtsPo-eM-3@gated-at.bofh.it>
On Thu, Feb 18, 2016 at 09:37:26AM -0800, Kees Cook wrote:
> On Thu, Feb 18, 2016 at 6:04 AM, Geliang Tang <geliangtang@163.com> wrote:
> > Like zlib compression in pstore, this patch added lzo and lz4
> > compression support so that users can have more options and better
> > compression ratio.
> >
> > The original code treats the compressed data together with the
> > uncompressed ECC correction notice by using zlib decompress. The
> > ECC correction notice is missing in the decompression process. The
> > treatment also makes lzo and lz4 not working. So I treat them
> > separately by using pstore_decompress() to treat the compressed
> > data, and memcpy() to treat the uncompressed ECC correction notice.
> >
> > Signed-off-by: Geliang Tang <geliangtang@163.com>
> 
> Thanks for the updates!
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> -Kees

Hi there,

It's been three months since this patch has been reviewed. But it hasn't
been replied yet by far. Can I please know if there's anything wrong with
it?

-Geliang Tang

> > ---
> > Changes in v2:
> >  revise the patch as Kees suggested:
> >  - drop 'depends on PSTORE' in config choice PSTORE_ZLIB/LZO/LZ4_COMPRESS;
> >  - set big_oops_buf_sz to 0 in free_zlib/lzo/lz4;
> >  - change zbackend[] to zbackend.
> > ---
> >  drivers/firmware/efi/efi-pstore.c |   1 +
> >  fs/pstore/Kconfig                 |  31 +++++-
> >  fs/pstore/platform.c              | 218 ++++++++++++++++++++++++++++++++++++--
> >  fs/pstore/ram.c                   |  10 +-
> >  include/linux/pstore.h            |   3 +-
> >  5 files changed, 248 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
> > index eac76a7..cd8c35f 100644
> > --- a/drivers/firmware/efi/efi-pstore.c
> > +++ b/drivers/firmware/efi/efi-pstore.c
> > @@ -210,6 +210,7 @@ static int efi_pstore_sysfs_entry_iter(void *data, struct efivar_entry **pos)
> >  static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
> >                                int *count, struct timespec *timespec,
> >                                char **buf, bool *compressed,
> > +                              ssize_t *ecc_notice_size,
> >                                struct pstore_info *psi)
> >  {
> >         struct pstore_read_data data;
> > diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
> > index 360ae43..be40813 100644
> > --- a/fs/pstore/Kconfig
> > +++ b/fs/pstore/Kconfig
> > @@ -1,8 +1,6 @@
> >  config PSTORE
> >         tristate "Persistent store support"
> >         default n
> > -       select ZLIB_DEFLATE
> > -       select ZLIB_INFLATE
> >         help
> >            This option enables generic access to platform level
> >            persistent storage via "pstore" filesystem that can
> > @@ -14,6 +12,35 @@ config PSTORE
> >            If you don't have a platform persistent store driver,
> >            say N.
> >
> > +choice
> > +        prompt "Choose compression algorithm"
> > +        depends on PSTORE
> > +        default PSTORE_ZLIB_COMPRESS
> > +        help
> > +          This option chooses compression algorithm.
> > +
> > +config PSTORE_ZLIB_COMPRESS
> > +        bool "ZLIB"
> > +        select ZLIB_DEFLATE
> > +        select ZLIB_INFLATE
> > +        help
> > +          This option enables ZLIB compression algorithm support.
> > +
> > +config PSTORE_LZO_COMPRESS
> > +        bool "LZO"
> > +        select LZO_COMPRESS
> > +        select LZO_DECOMPRESS
> > +        help
> > +          This option enables LZO compression algorithm support.
> > +
> > +config PSTORE_LZ4_COMPRESS
> > +        bool "LZ4"
> > +        select LZ4_COMPRESS
> > +        select LZ4_DECOMPRESS
> > +        help
> > +          This option enables LZ4 compression algorithm support.
> > +endchoice
> > +
> >  config PSTORE_CONSOLE
> >         bool "Log kernel console messages"
> >         depends on PSTORE
> > diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> > index 588461b..7246d50 100644
> > --- a/fs/pstore/platform.c
> > +++ b/fs/pstore/platform.c
> > @@ -28,7 +28,15 @@
> >  #include <linux/console.h>
> >  #include <linux/module.h>
> >  #include <linux/pstore.h>
> > +#ifdef CONFIG_PSTORE_ZLIB_COMPRESS
> >  #include <linux/zlib.h>
> > +#endif
> > +#ifdef CONFIG_PSTORE_LZO_COMPRESS
> > +#include <linux/lzo.h>
> > +#endif
> > +#ifdef CONFIG_PSTORE_LZ4_COMPRESS
> > +#include <linux/lz4.h>
> > +#endif
> >  #include <linux/string.h>
> >  #include <linux/timer.h>
> >  #include <linux/slab.h>
> > @@ -69,10 +77,23 @@ struct pstore_info *psinfo;
> >  static char *backend;
> >
> >  /* Compression parameters */
> > +#ifdef CONFIG_PSTORE_ZLIB_COMPRESS
> >  #define COMPR_LEVEL 6
> >  #define WINDOW_BITS 12
> >  #define MEM_LEVEL 4
> >  static struct z_stream_s stream;
> > +#else
> > +static unsigned char *workspace;
> > +#endif
> > +
> > +struct pstore_zbackend {
> > +       int (*compress)(const void *in, void *out, size_t inlen, size_t outlen);
> > +       int (*decompress)(void *in, void *out, size_t inlen, size_t outlen);
> > +       void (*allocate)(void);
> > +       void (*free)(void);
> > +
> > +       const char *name;
> > +};
> >
> >  static char *big_oops_buf;
> >  static size_t big_oops_buf_sz;
> > @@ -129,9 +150,9 @@ bool pstore_cannot_block_path(enum kmsg_dump_reason reason)
> >  }
> >  EXPORT_SYMBOL_GPL(pstore_cannot_block_path);
> >
> > +#ifdef CONFIG_PSTORE_ZLIB_COMPRESS
> >  /* Derived from logfs_compress() */
> > -static int pstore_compress(const void *in, void *out, size_t inlen,
> > -                                                       size_t outlen)
> > +static int compress_zlib(const void *in, void *out, size_t inlen, size_t outlen)
> >  {
> >         int err, ret;
> >
> > @@ -165,7 +186,7 @@ error:
> >  }
> >
> >  /* Derived from logfs_uncompress */
> > -static int pstore_decompress(void *in, void *out, size_t inlen, size_t outlen)
> > +static int decompress_zlib(void *in, void *out, size_t inlen, size_t outlen)
> >  {
> >         int err, ret;
> >
> > @@ -194,7 +215,7 @@ error:
> >         return ret;
> >  }
> >
> > -static void allocate_buf_for_compression(void)
> > +static void allocate_zlib(void)
> >  {
> >         size_t size;
> >         size_t cmpr;
> > @@ -237,12 +258,190 @@ static void allocate_buf_for_compression(void)
> >
> >  }
> >
> > -static void free_buf_for_compression(void)
> > +static void free_zlib(void)
> >  {
> >         kfree(stream.workspace);
> >         stream.workspace = NULL;
> >         kfree(big_oops_buf);
> >         big_oops_buf = NULL;
> > +       big_oops_buf_sz = 0;
> > +}
> > +
> > +static struct pstore_zbackend backend_zlib = {
> > +       .compress       = compress_zlib,
> > +       .decompress     = decompress_zlib,
> > +       .allocate       = allocate_zlib,
> > +       .free           = free_zlib,
> > +       .name           = "zlib",
> > +};
> > +#endif
> > +
> > +#ifdef CONFIG_PSTORE_LZO_COMPRESS
> > +static int compress_lzo(const void *in, void *out, size_t inlen, size_t outlen)
> > +{
> > +       int ret;
> > +
> > +       ret = lzo1x_1_compress(in, inlen, out, &outlen, workspace);
> > +       if (ret != LZO_E_OK) {
> > +               pr_err("lzo_compress error, ret = %d!\n", ret);
> > +               return -EIO;
> > +       }
> > +
> > +       return outlen;
> > +}
> > +
> > +static int decompress_lzo(void *in, void *out, size_t inlen, size_t outlen)
> > +{
> > +       int ret;
> > +
> > +       ret = lzo1x_decompress_safe(in, inlen, out, &outlen);
> > +       if (ret != LZO_E_OK) {
> > +               pr_err("lzo_decompress error, ret = %d!\n", ret);
> > +               return -EIO;
> > +       }
> > +
> > +       return outlen;
> > +}
> > +
> > +static void allocate_lzo(void)
> > +{
> > +       big_oops_buf_sz = lzo1x_worst_compress(psinfo->bufsize);
> > +       big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
> > +       if (big_oops_buf) {
> > +               workspace = kmalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
> > +               if (!workspace) {
> > +                       pr_err("No memory for compression workspace; skipping compression\n");
> > +                       kfree(big_oops_buf);
> > +                       big_oops_buf = NULL;
> > +               }
> > +       } else {
> > +               pr_err("No memory for uncompressed data; skipping compression\n");
> > +               workspace = NULL;
> > +       }
> > +}
> > +
> > +static void free_lzo(void)
> > +{
> > +       kfree(workspace);
> > +       kfree(big_oops_buf);
> > +       big_oops_buf = NULL;
> > +       big_oops_buf_sz = 0;
> > +}
> > +
> > +static struct pstore_zbackend backend_lzo = {
> > +       .compress       = compress_lzo,
> > +       .decompress     = decompress_lzo,
> > +       .allocate       = allocate_lzo,
> > +       .free           = free_lzo,
> > +       .name           = "lzo",
> > +};
> > +#endif
> > +
> > +#ifdef CONFIG_PSTORE_LZ4_COMPRESS
> > +static int compress_lz4(const void *in, void *out, size_t inlen, size_t outlen)
> > +{
> > +       int ret;
> > +
> > +       ret = lz4_compress(in, inlen, out, &outlen, workspace);
> > +       if (ret) {
> > +               pr_err("lz4_compress error, ret = %d!\n", ret);
> > +               return -EIO;
> > +       }
> > +
> > +       return outlen;
> > +}
> > +
> > +static int decompress_lz4(void *in, void *out, size_t inlen, size_t outlen)
> > +{
> > +       int ret;
> > +
> > +       ret = lz4_decompress_unknownoutputsize(in, inlen, out, &outlen);
> > +       if (ret) {
> > +               pr_err("lz4_decompress error, ret = %d!\n", ret);
> > +               return -EIO;
> > +       }
> > +
> > +       return outlen;
> > +}
> > +
> > +static void allocate_lz4(void)
> > +{
> > +       big_oops_buf_sz = lz4_compressbound(psinfo->bufsize);
> > +       big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
> > +       if (big_oops_buf) {
> > +               workspace = kmalloc(LZ4_MEM_COMPRESS, GFP_KERNEL);
> > +               if (!workspace) {
> > +                       pr_err("No memory for compression workspace; skipping compression\n");
> > +                       kfree(big_oops_buf);
> > +                       big_oops_buf = NULL;
> > +               }
> > +       } else {
> > +               pr_err("No memory for uncompressed data; skipping compression\n");
> > +               workspace = NULL;
> > +       }
> > +}
> > +
> > +static void free_lz4(void)
> > +{
> > +       kfree(workspace);
> > +       kfree(big_oops_buf);
> > +       big_oops_buf = NULL;
> > +       big_oops_buf_sz = 0;
> > +}
> > +
> > +static struct pstore_zbackend backend_lz4 = {
> > +       .compress       = compress_lz4,
> > +       .decompress     = decompress_lz4,
> > +       .allocate       = allocate_lz4,
> > +       .free           = free_lz4,
> > +       .name           = "lz4",
> > +};
> > +#endif
> > +
> > +static struct pstore_zbackend *zbackend =
> > +#if defined(CONFIG_PSTORE_ZLIB_COMPRESS)
> > +       &backend_zlib;
> > +#elif defined(CONFIG_PSTORE_LZO_COMPRESS)
> > +       &backend_lzo;
> > +#elif defined(CONFIG_PSTORE_LZ4_COMPRESS)
> > +       &backend_lz4;
> > +#else
> > +       NULL;
> > +#endif
> > +
> > +static int pstore_compress(const void *in, void *out,
> > +                          size_t inlen, size_t outlen)
> > +{
> > +       if (zbackend)
> > +               return zbackend->compress(in, out, inlen, outlen);
> > +       else
> > +               return -EIO;
> > +}
> > +
> > +static int pstore_decompress(void *in, void *out, size_t inlen, size_t outlen)
> > +{
> > +       if (zbackend)
> > +               return zbackend->decompress(in, out, inlen, outlen);
> > +       else
> > +               return -EIO;
> > +}
> > +
> > +static void allocate_buf_for_compression(void)
> > +{
> > +       if (zbackend) {
> > +               pr_info("using %s compression\n", zbackend->name);
> > +               zbackend->allocate();
> > +       } else {
> > +               pr_err("allocate compression buffer error!\n");
> > +       }
> > +}
> > +
> > +static void free_buf_for_compression(void)
> > +{
> > +       if (zbackend)
> > +               zbackend->free();
> > +       else
> > +               pr_err("free compression buffer error!\n");
> >  }
> >
> >  /*
> > @@ -527,6 +726,7 @@ void pstore_get_records(int quiet)
> >         int                     failed = 0, rc;
> >         bool                    compressed;
> >         int                     unzipped_len = -1;
> > +       ssize_t                 ecc_notice_size = 0;
> >
> >         if (!psi)
> >                 return;
> > @@ -536,7 +736,7 @@ void pstore_get_records(int quiet)
> >                 goto out;
> >
> >         while ((size = psi->read(&id, &type, &count, &time, &buf, &compressed,
> > -                               psi)) > 0) {
> > +                                &ecc_notice_size, psi)) > 0) {
> >                 if (compressed && (type == PSTORE_TYPE_DMESG)) {
> >                         if (big_oops_buf)
> >                                 unzipped_len = pstore_decompress(buf,
> > @@ -544,6 +744,9 @@ void pstore_get_records(int quiet)
> >                                                         big_oops_buf_sz);
> >
> >                         if (unzipped_len > 0) {
> > +                               if (ecc_notice_size)
> > +                                       memcpy(big_oops_buf + unzipped_len,
> > +                                              buf + size, ecc_notice_size);
> >                                 kfree(buf);
> >                                 buf = big_oops_buf;
> >                                 size = unzipped_len;
> > @@ -555,7 +758,8 @@ void pstore_get_records(int quiet)
> >                         }
> >                 }
> >                 rc = pstore_mkfile(type, psi->name, id, count, buf,
> > -                                 compressed, (size_t)size, time, psi);
> > +                                  compressed, size + ecc_notice_size,
> > +                                  time, psi);
> >                 if (unzipped_len < 0) {
> >                         /* Free buffer other than big oops */
> >                         kfree(buf);
> > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> > index 319c3a6..11ec024 100644
> > --- a/fs/pstore/ram.c
> > +++ b/fs/pstore/ram.c
> > @@ -181,10 +181,10 @@ static bool prz_ok(struct persistent_ram_zone *prz)
> >  static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
> >                                    int *count, struct timespec *time,
> >                                    char **buf, bool *compressed,
> > +                                  ssize_t *ecc_notice_size,
> >                                    struct pstore_info *psi)
> >  {
> >         ssize_t size;
> > -       ssize_t ecc_notice_size;
> >         struct ramoops_context *cxt = psi->data;
> >         struct persistent_ram_zone *prz = NULL;
> >         int header_length = 0;
> > @@ -229,16 +229,16 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
> >         size = persistent_ram_old_size(prz) - header_length;
> >
> >         /* ECC correction notice */
> > -       ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
> > +       *ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
> >
> > -       *buf = kmalloc(size + ecc_notice_size + 1, GFP_KERNEL);
> > +       *buf = kmalloc(size + *ecc_notice_size + 1, GFP_KERNEL);
> >         if (*buf == NULL)
> >                 return -ENOMEM;
> >
> >         memcpy(*buf, (char *)persistent_ram_old(prz) + header_length, size);
> > -       persistent_ram_ecc_string(prz, *buf + size, ecc_notice_size + 1);
> > +       persistent_ram_ecc_string(prz, *buf + size, *ecc_notice_size + 1);
> >
> > -       return size + ecc_notice_size;
> > +       return size;
> >  }
> >
> >  static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz,
> > diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> > index 831479f..899e95e 100644
> > --- a/include/linux/pstore.h
> > +++ b/include/linux/pstore.h
> > @@ -58,7 +58,8 @@ struct pstore_info {
> >         int             (*close)(struct pstore_info *psi);
> >         ssize_t         (*read)(u64 *id, enum pstore_type_id *type,
> >                         int *count, struct timespec *time, char **buf,
> > -                       bool *compressed, struct pstore_info *psi);
> > +                       bool *compressed, ssize_t *ecc_notice_size,
> > +                       struct pstore_info *psi);
> >         int             (*write)(enum pstore_type_id type,
> >                         enum kmsg_dump_reason reason, u64 *id,
> >                         unsigned int part, int count, bool compressed,
> > --
> > 2.5.0
> >
> >
> 
> 
> 
> -- 
> Kees Cook
> Chrome OS & Brillo Security

[toc] | [next] | [standalone]


#1392448

FromKees Cook <keescook@chromium.org>
Date2016-05-02 20:40 +0200
Message-ID<ruqVe-dJ-59@gated-at.bofh.it>
In reply to#1391563
On Fri, Apr 29, 2016 at 7:26 PM, Geliang Tang <geliangtang@163.com> wrote:
> On Thu, Feb 18, 2016 at 09:37:26AM -0800, Kees Cook wrote:
>> On Thu, Feb 18, 2016 at 6:04 AM, Geliang Tang <geliangtang@163.com> wrote:
>> > Like zlib compression in pstore, this patch added lzo and lz4
>> > compression support so that users can have more options and better
>> > compression ratio.
>> >
>> > The original code treats the compressed data together with the
>> > uncompressed ECC correction notice by using zlib decompress. The
>> > ECC correction notice is missing in the decompression process. The
>> > treatment also makes lzo and lz4 not working. So I treat them
>> > separately by using pstore_decompress() to treat the compressed
>> > data, and memcpy() to treat the uncompressed ECC correction notice.
>> >
>> > Signed-off-by: Geliang Tang <geliangtang@163.com>
>>
>> Thanks for the updates!
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>
>> -Kees
>
> Hi there,
>
> It's been three months since this patch has been reviewed. But it hasn't
> been replied yet by far. Can I please know if there's anything wrong with
> it?

Tony, should I take over the pstore tree? Do you have any testing
procedures I could use? My testing is rather manual at the moment.

-Kees

>
> -Geliang Tang
>
>> > ---
>> > Changes in v2:
>> >  revise the patch as Kees suggested:
>> >  - drop 'depends on PSTORE' in config choice PSTORE_ZLIB/LZO/LZ4_COMPRESS;
>> >  - set big_oops_buf_sz to 0 in free_zlib/lzo/lz4;
>> >  - change zbackend[] to zbackend.
>> > ---
>> >  drivers/firmware/efi/efi-pstore.c |   1 +
>> >  fs/pstore/Kconfig                 |  31 +++++-
>> >  fs/pstore/platform.c              | 218 ++++++++++++++++++++++++++++++++++++--
>> >  fs/pstore/ram.c                   |  10 +-
>> >  include/linux/pstore.h            |   3 +-
>> >  5 files changed, 248 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
>> > index eac76a7..cd8c35f 100644
>> > --- a/drivers/firmware/efi/efi-pstore.c
>> > +++ b/drivers/firmware/efi/efi-pstore.c
>> > @@ -210,6 +210,7 @@ static int efi_pstore_sysfs_entry_iter(void *data, struct efivar_entry **pos)
>> >  static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
>> >                                int *count, struct timespec *timespec,
>> >                                char **buf, bool *compressed,
>> > +                              ssize_t *ecc_notice_size,
>> >                                struct pstore_info *psi)
>> >  {
>> >         struct pstore_read_data data;
>> > diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
>> > index 360ae43..be40813 100644
>> > --- a/fs/pstore/Kconfig
>> > +++ b/fs/pstore/Kconfig
>> > @@ -1,8 +1,6 @@
>> >  config PSTORE
>> >         tristate "Persistent store support"
>> >         default n
>> > -       select ZLIB_DEFLATE
>> > -       select ZLIB_INFLATE
>> >         help
>> >            This option enables generic access to platform level
>> >            persistent storage via "pstore" filesystem that can
>> > @@ -14,6 +12,35 @@ config PSTORE
>> >            If you don't have a platform persistent store driver,
>> >            say N.
>> >
>> > +choice
>> > +        prompt "Choose compression algorithm"
>> > +        depends on PSTORE
>> > +        default PSTORE_ZLIB_COMPRESS
>> > +        help
>> > +          This option chooses compression algorithm.
>> > +
>> > +config PSTORE_ZLIB_COMPRESS
>> > +        bool "ZLIB"
>> > +        select ZLIB_DEFLATE
>> > +        select ZLIB_INFLATE
>> > +        help
>> > +          This option enables ZLIB compression algorithm support.
>> > +
>> > +config PSTORE_LZO_COMPRESS
>> > +        bool "LZO"
>> > +        select LZO_COMPRESS
>> > +        select LZO_DECOMPRESS
>> > +        help
>> > +          This option enables LZO compression algorithm support.
>> > +
>> > +config PSTORE_LZ4_COMPRESS
>> > +        bool "LZ4"
>> > +        select LZ4_COMPRESS
>> > +        select LZ4_DECOMPRESS
>> > +        help
>> > +          This option enables LZ4 compression algorithm support.
>> > +endchoice
>> > +
>> >  config PSTORE_CONSOLE
>> >         bool "Log kernel console messages"
>> >         depends on PSTORE
>> > diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
>> > index 588461b..7246d50 100644
>> > --- a/fs/pstore/platform.c
>> > +++ b/fs/pstore/platform.c
>> > @@ -28,7 +28,15 @@
>> >  #include <linux/console.h>
>> >  #include <linux/module.h>
>> >  #include <linux/pstore.h>
>> > +#ifdef CONFIG_PSTORE_ZLIB_COMPRESS
>> >  #include <linux/zlib.h>
>> > +#endif
>> > +#ifdef CONFIG_PSTORE_LZO_COMPRESS
>> > +#include <linux/lzo.h>
>> > +#endif
>> > +#ifdef CONFIG_PSTORE_LZ4_COMPRESS
>> > +#include <linux/lz4.h>
>> > +#endif
>> >  #include <linux/string.h>
>> >  #include <linux/timer.h>
>> >  #include <linux/slab.h>
>> > @@ -69,10 +77,23 @@ struct pstore_info *psinfo;
>> >  static char *backend;
>> >
>> >  /* Compression parameters */
>> > +#ifdef CONFIG_PSTORE_ZLIB_COMPRESS
>> >  #define COMPR_LEVEL 6
>> >  #define WINDOW_BITS 12
>> >  #define MEM_LEVEL 4
>> >  static struct z_stream_s stream;
>> > +#else
>> > +static unsigned char *workspace;
>> > +#endif
>> > +
>> > +struct pstore_zbackend {
>> > +       int (*compress)(const void *in, void *out, size_t inlen, size_t outlen);
>> > +       int (*decompress)(void *in, void *out, size_t inlen, size_t outlen);
>> > +       void (*allocate)(void);
>> > +       void (*free)(void);
>> > +
>> > +       const char *name;
>> > +};
>> >
>> >  static char *big_oops_buf;
>> >  static size_t big_oops_buf_sz;
>> > @@ -129,9 +150,9 @@ bool pstore_cannot_block_path(enum kmsg_dump_reason reason)
>> >  }
>> >  EXPORT_SYMBOL_GPL(pstore_cannot_block_path);
>> >
>> > +#ifdef CONFIG_PSTORE_ZLIB_COMPRESS
>> >  /* Derived from logfs_compress() */
>> > -static int pstore_compress(const void *in, void *out, size_t inlen,
>> > -                                                       size_t outlen)
>> > +static int compress_zlib(const void *in, void *out, size_t inlen, size_t outlen)
>> >  {
>> >         int err, ret;
>> >
>> > @@ -165,7 +186,7 @@ error:
>> >  }
>> >
>> >  /* Derived from logfs_uncompress */
>> > -static int pstore_decompress(void *in, void *out, size_t inlen, size_t outlen)
>> > +static int decompress_zlib(void *in, void *out, size_t inlen, size_t outlen)
>> >  {
>> >         int err, ret;
>> >
>> > @@ -194,7 +215,7 @@ error:
>> >         return ret;
>> >  }
>> >
>> > -static void allocate_buf_for_compression(void)
>> > +static void allocate_zlib(void)
>> >  {
>> >         size_t size;
>> >         size_t cmpr;
>> > @@ -237,12 +258,190 @@ static void allocate_buf_for_compression(void)
>> >
>> >  }
>> >
>> > -static void free_buf_for_compression(void)
>> > +static void free_zlib(void)
>> >  {
>> >         kfree(stream.workspace);
>> >         stream.workspace = NULL;
>> >         kfree(big_oops_buf);
>> >         big_oops_buf = NULL;
>> > +       big_oops_buf_sz = 0;
>> > +}
>> > +
>> > +static struct pstore_zbackend backend_zlib = {
>> > +       .compress       = compress_zlib,
>> > +       .decompress     = decompress_zlib,
>> > +       .allocate       = allocate_zlib,
>> > +       .free           = free_zlib,
>> > +       .name           = "zlib",
>> > +};
>> > +#endif
>> > +
>> > +#ifdef CONFIG_PSTORE_LZO_COMPRESS
>> > +static int compress_lzo(const void *in, void *out, size_t inlen, size_t outlen)
>> > +{
>> > +       int ret;
>> > +
>> > +       ret = lzo1x_1_compress(in, inlen, out, &outlen, workspace);
>> > +       if (ret != LZO_E_OK) {
>> > +               pr_err("lzo_compress error, ret = %d!\n", ret);
>> > +               return -EIO;
>> > +       }
>> > +
>> > +       return outlen;
>> > +}
>> > +
>> > +static int decompress_lzo(void *in, void *out, size_t inlen, size_t outlen)
>> > +{
>> > +       int ret;
>> > +
>> > +       ret = lzo1x_decompress_safe(in, inlen, out, &outlen);
>> > +       if (ret != LZO_E_OK) {
>> > +               pr_err("lzo_decompress error, ret = %d!\n", ret);
>> > +               return -EIO;
>> > +       }
>> > +
>> > +       return outlen;
>> > +}
>> > +
>> > +static void allocate_lzo(void)
>> > +{
>> > +       big_oops_buf_sz = lzo1x_worst_compress(psinfo->bufsize);
>> > +       big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
>> > +       if (big_oops_buf) {
>> > +               workspace = kmalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
>> > +               if (!workspace) {
>> > +                       pr_err("No memory for compression workspace; skipping compression\n");
>> > +                       kfree(big_oops_buf);
>> > +                       big_oops_buf = NULL;
>> > +               }
>> > +       } else {
>> > +               pr_err("No memory for uncompressed data; skipping compression\n");
>> > +               workspace = NULL;
>> > +       }
>> > +}
>> > +
>> > +static void free_lzo(void)
>> > +{
>> > +       kfree(workspace);
>> > +       kfree(big_oops_buf);
>> > +       big_oops_buf = NULL;
>> > +       big_oops_buf_sz = 0;
>> > +}
>> > +
>> > +static struct pstore_zbackend backend_lzo = {
>> > +       .compress       = compress_lzo,
>> > +       .decompress     = decompress_lzo,
>> > +       .allocate       = allocate_lzo,
>> > +       .free           = free_lzo,
>> > +       .name           = "lzo",
>> > +};
>> > +#endif
>> > +
>> > +#ifdef CONFIG_PSTORE_LZ4_COMPRESS
>> > +static int compress_lz4(const void *in, void *out, size_t inlen, size_t outlen)
>> > +{
>> > +       int ret;
>> > +
>> > +       ret = lz4_compress(in, inlen, out, &outlen, workspace);
>> > +       if (ret) {
>> > +               pr_err("lz4_compress error, ret = %d!\n", ret);
>> > +               return -EIO;
>> > +       }
>> > +
>> > +       return outlen;
>> > +}
>> > +
>> > +static int decompress_lz4(void *in, void *out, size_t inlen, size_t outlen)
>> > +{
>> > +       int ret;
>> > +
>> > +       ret = lz4_decompress_unknownoutputsize(in, inlen, out, &outlen);
>> > +       if (ret) {
>> > +               pr_err("lz4_decompress error, ret = %d!\n", ret);
>> > +               return -EIO;
>> > +       }
>> > +
>> > +       return outlen;
>> > +}
>> > +
>> > +static void allocate_lz4(void)
>> > +{
>> > +       big_oops_buf_sz = lz4_compressbound(psinfo->bufsize);
>> > +       big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
>> > +       if (big_oops_buf) {
>> > +               workspace = kmalloc(LZ4_MEM_COMPRESS, GFP_KERNEL);
>> > +               if (!workspace) {
>> > +                       pr_err("No memory for compression workspace; skipping compression\n");
>> > +                       kfree(big_oops_buf);
>> > +                       big_oops_buf = NULL;
>> > +               }
>> > +       } else {
>> > +               pr_err("No memory for uncompressed data; skipping compression\n");
>> > +               workspace = NULL;
>> > +       }
>> > +}
>> > +
>> > +static void free_lz4(void)
>> > +{
>> > +       kfree(workspace);
>> > +       kfree(big_oops_buf);
>> > +       big_oops_buf = NULL;
>> > +       big_oops_buf_sz = 0;
>> > +}
>> > +
>> > +static struct pstore_zbackend backend_lz4 = {
>> > +       .compress       = compress_lz4,
>> > +       .decompress     = decompress_lz4,
>> > +       .allocate       = allocate_lz4,
>> > +       .free           = free_lz4,
>> > +       .name           = "lz4",
>> > +};
>> > +#endif
>> > +
>> > +static struct pstore_zbackend *zbackend =
>> > +#if defined(CONFIG_PSTORE_ZLIB_COMPRESS)
>> > +       &backend_zlib;
>> > +#elif defined(CONFIG_PSTORE_LZO_COMPRESS)
>> > +       &backend_lzo;
>> > +#elif defined(CONFIG_PSTORE_LZ4_COMPRESS)
>> > +       &backend_lz4;
>> > +#else
>> > +       NULL;
>> > +#endif
>> > +
>> > +static int pstore_compress(const void *in, void *out,
>> > +                          size_t inlen, size_t outlen)
>> > +{
>> > +       if (zbackend)
>> > +               return zbackend->compress(in, out, inlen, outlen);
>> > +       else
>> > +               return -EIO;
>> > +}
>> > +
>> > +static int pstore_decompress(void *in, void *out, size_t inlen, size_t outlen)
>> > +{
>> > +       if (zbackend)
>> > +               return zbackend->decompress(in, out, inlen, outlen);
>> > +       else
>> > +               return -EIO;
>> > +}
>> > +
>> > +static void allocate_buf_for_compression(void)
>> > +{
>> > +       if (zbackend) {
>> > +               pr_info("using %s compression\n", zbackend->name);
>> > +               zbackend->allocate();
>> > +       } else {
>> > +               pr_err("allocate compression buffer error!\n");
>> > +       }
>> > +}
>> > +
>> > +static void free_buf_for_compression(void)
>> > +{
>> > +       if (zbackend)
>> > +               zbackend->free();
>> > +       else
>> > +               pr_err("free compression buffer error!\n");
>> >  }
>> >
>> >  /*
>> > @@ -527,6 +726,7 @@ void pstore_get_records(int quiet)
>> >         int                     failed = 0, rc;
>> >         bool                    compressed;
>> >         int                     unzipped_len = -1;
>> > +       ssize_t                 ecc_notice_size = 0;
>> >
>> >         if (!psi)
>> >                 return;
>> > @@ -536,7 +736,7 @@ void pstore_get_records(int quiet)
>> >                 goto out;
>> >
>> >         while ((size = psi->read(&id, &type, &count, &time, &buf, &compressed,
>> > -                               psi)) > 0) {
>> > +                                &ecc_notice_size, psi)) > 0) {
>> >                 if (compressed && (type == PSTORE_TYPE_DMESG)) {
>> >                         if (big_oops_buf)
>> >                                 unzipped_len = pstore_decompress(buf,
>> > @@ -544,6 +744,9 @@ void pstore_get_records(int quiet)
>> >                                                         big_oops_buf_sz);
>> >
>> >                         if (unzipped_len > 0) {
>> > +                               if (ecc_notice_size)
>> > +                                       memcpy(big_oops_buf + unzipped_len,
>> > +                                              buf + size, ecc_notice_size);
>> >                                 kfree(buf);
>> >                                 buf = big_oops_buf;
>> >                                 size = unzipped_len;
>> > @@ -555,7 +758,8 @@ void pstore_get_records(int quiet)
>> >                         }
>> >                 }
>> >                 rc = pstore_mkfile(type, psi->name, id, count, buf,
>> > -                                 compressed, (size_t)size, time, psi);
>> > +                                  compressed, size + ecc_notice_size,
>> > +                                  time, psi);
>> >                 if (unzipped_len < 0) {
>> >                         /* Free buffer other than big oops */
>> >                         kfree(buf);
>> > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>> > index 319c3a6..11ec024 100644
>> > --- a/fs/pstore/ram.c
>> > +++ b/fs/pstore/ram.c
>> > @@ -181,10 +181,10 @@ static bool prz_ok(struct persistent_ram_zone *prz)
>> >  static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
>> >                                    int *count, struct timespec *time,
>> >                                    char **buf, bool *compressed,
>> > +                                  ssize_t *ecc_notice_size,
>> >                                    struct pstore_info *psi)
>> >  {
>> >         ssize_t size;
>> > -       ssize_t ecc_notice_size;
>> >         struct ramoops_context *cxt = psi->data;
>> >         struct persistent_ram_zone *prz = NULL;
>> >         int header_length = 0;
>> > @@ -229,16 +229,16 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
>> >         size = persistent_ram_old_size(prz) - header_length;
>> >
>> >         /* ECC correction notice */
>> > -       ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
>> > +       *ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
>> >
>> > -       *buf = kmalloc(size + ecc_notice_size + 1, GFP_KERNEL);
>> > +       *buf = kmalloc(size + *ecc_notice_size + 1, GFP_KERNEL);
>> >         if (*buf == NULL)
>> >                 return -ENOMEM;
>> >
>> >         memcpy(*buf, (char *)persistent_ram_old(prz) + header_length, size);
>> > -       persistent_ram_ecc_string(prz, *buf + size, ecc_notice_size + 1);
>> > +       persistent_ram_ecc_string(prz, *buf + size, *ecc_notice_size + 1);
>> >
>> > -       return size + ecc_notice_size;
>> > +       return size;
>> >  }
>> >
>> >  static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz,
>> > diff --git a/include/linux/pstore.h b/include/linux/pstore.h
>> > index 831479f..899e95e 100644
>> > --- a/include/linux/pstore.h
>> > +++ b/include/linux/pstore.h
>> > @@ -58,7 +58,8 @@ struct pstore_info {
>> >         int             (*close)(struct pstore_info *psi);
>> >         ssize_t         (*read)(u64 *id, enum pstore_type_id *type,
>> >                         int *count, struct timespec *time, char **buf,
>> > -                       bool *compressed, struct pstore_info *psi);
>> > +                       bool *compressed, ssize_t *ecc_notice_size,
>> > +                       struct pstore_info *psi);
>> >         int             (*write)(enum pstore_type_id type,
>> >                         enum kmsg_dump_reason reason, u64 *id,
>> >                         unsigned int part, int count, bool compressed,
>> > --
>> > 2.5.0
>> >
>> >
>>
>>
>>
>> --
>> Kees Cook
>> Chrome OS & Brillo Security
>



-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]


#1392454

From"Luck, Tony" <tony.luck@intel.com>
Date2016-05-02 20:50 +0200
Message-ID<rur4S-iN-25@gated-at.bofh.it>
In reply to#1392448
> Tony, should I take over the pstore tree? Do you have any testing
> procedures I could use? My testing is rather manual at the moment.

Kees,

Sure - I seem to be bad about keeping track of stuff here.

I don't have any good tests ... just manually crash a machine and
make sure things show up in /sys/fs/pstore

-Tony


[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web