[Crash-utility] [PATCH v3] diskdump: Add support for reading dumpfiles compressed by Zstandard

HAGIO KAZUHITO(萩尾 一仁) k-hagio-ab at nec.com
Mon Sep 27 00:36:15 UTC 2021


Hi Lianbo,

Thanks for the review.

-----Original Message-----
> > --- a/diskdump.c
> > +++ b/diskdump.c
> > @@ -96,6 +96,10 @@ static struct diskdump_data **dd_list = NULL;
> >  static int num_dd = 0;
> >  static int num_dumpfiles = 0;
> >
> > +#ifdef ZSTD
> > +static ZSTD_DCtx *dctx = NULL;
> > +#endif
> > +
>
> Would it be better to move the above definition to cache_page()? Because it is
> not used in other functions, and the behavior is the same.

OK, now we can move this to cache_pages(), too.

I found a slight fix, will post v4.

>
> >  int dumpfile_is_split(void)
> >  {
> >         return KDUMP_SPLIT();
> > @@ -1001,6 +1005,9 @@ is_diskdump(char *file)
> >  #ifdef SNAPPY
> >         dd->flags |= SNAPPY_SUPPORTED;
> >  #endif
> > +#ifdef ZSTD
> > +       dd->flags |= ZSTD_SUPPORTED;
> > +#endif
> >
> >         pc->read_vmcoreinfo = vmcoreinfo_read_string;
> >
> > @@ -1251,6 +1258,33 @@ cache_page(physaddr_t paddr)
> >                               ret);
> >                         return READ_ERROR;
> >                 }
> > +#endif
> > +       } else if (pd.flags & DUMP_DH_COMPRESSED_ZSTD) {
> > +
> > +               if (!(dd->flags & ZSTD_SUPPORTED)) {
> > +                       error(INFO, "%s: uncompess failed: no zstd compression support\n",
> > +                               DISKDUMP_VALID() ? "diskdump" : "compressed kdump");
> > +                       return READ_ERROR;
> > +               }
> > +#ifdef ZSTD
> > +               if (!dctx)
> > +                       dctx = ZSTD_createDCtx();
> > +
>
> Usually these two functions ZSTD_createDCtx()/ZSTC_free() are required
> to be called in pairs, but this(static definition) seems to simplify
> the code.

Yes, crash continues using a dctx in a session to reduce the workload of
allocation and release, and I think there will be no big benefit with
having dd->dctx and making efforts to free it when crash exits.

Thanks,
Kazu






More information about the Crash-utility mailing list