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
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
