On 2024/03/26 15:38, Tang Yulong wrote:
> Hi, Kazu
> 
> Thank you for your review.
> 
>> On 2024/03/12 17:25, Yulong TANG 汤玉龙 wrote:
>>
>> Thanks for the update.
>>
>>
>> why are this ifdef and lzo_init() etc. needed?  I think we do not use
>> the lzo library for lzo-rle.  maybe I'm missing something..
> 
> Yes, you are right. we do not use the lzo library for lzo-rle.
>   the "lzorle_decompress_safe" function should be directly used here.
> 
>>
>>
>> There is no need to check this every call, how about making this static?
>>    for example:
>>
>> static int efficient_unaligned_access = -1;
>> if (efficient_unaligned_access == -1) {
>> #if defined(ARM) || defined(ARM64) || defined(X86) || defined(X86_64) ||
>> defined(PPC) || defined(PPC64) || defined(S390)|| defined(S390X)
>>      efficient_unaligned_access = TRUE;
>> #else
>>      efficient_unaligned_access = FALSE;
>> #endif
>>      if ((kt->ikconfig_flags & IKCONFIG_AVAIL) &&
>>          (get_kernel_config("CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS", NULL)
>> == IKCONFIG_Y)
>>              efficient_unaligned_access = TRUE;
>> }
> 
> This is a good approach to avoid checking every call,
> 
> I've made those modifications we talked about, tested, and it's looking good.
> Anything else we should optimize?

Thank you for testing.  a nit, we use 'ulong' for unsigned long usually, 
otherwise looks fine to me.

Thanks,
Kazu

> 
> Thanks,
> Yulong
> 
> --- a/diskdump.c
> +++ b/diskdump.c
> @@ -3070,20 +3070,7 @@ try_zram_decompress(ulonglong pte_val, unsigned char 
> *buf, ulong len, ulonglong
>                  return 0;
>   #endif
>          } else if (STREQ(name, "lzo-rle")) {
> -#ifdef LZO
> -               if (!(dd->flags & LZO_SUPPORTED)) {
> -                       if (lzo_init() == LZO_E_OK)
> -                               dd->flags |= LZO_SUPPORTED;
> -                       else
> -                               return 0;
> -               }
>                  decompressor = (void *)&lzorle_decompress_safe;
> -#else
> -               error(WARNING,
> -                     "zram decompress error: this executable needs to be 
> built"
> -                     " with lzo-rle library\n");
> -               return 0;
> -#endif
>          } else { /* todo: support more compressor */
>                  error(WARNING, "only the lzo compressor is supported\n");
>                  return 0;
> 
> --- a/lzorle_decompress.c
> +++ b/lzorle_decompress.c
> @@ -50,16 +50,18 @@ int lzorle_decompress_safe(const unsigned char *in, 
> unsigned long in_len,
>   
>          unsigned char bitstream_version;
>   
> -       bool efficient_unaligned_access;
> +       static int efficient_unaligned_access = -1;
>   
> +       if (efficient_unaligned_access == -1) {
>   #if defined(ARM) || defined(ARM64) || defined(X86) || defined(X86_64) || 
> defined(PPC) || defined(PPC64) || defined(S390)|| defined(S390X)
> -       efficient_unaligned_access = true;
> +               efficient_unaligned_access = TRUE;
>   #else
> -       efficient_unaligned_access = false;
> +               efficient_unaligned_access = FALSE;
>   #endif
>   
> -       if (kt->ikconfig_flags & IKCONFIG_AVAIL)
> -               efficient_unaligned_access = 
> (get_kernel_config("CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS", NULL) == 
> IKCONFIG_Y);
> +               if ((kt->ikconfig_flags & IKCONFIG_AVAIL) && 
> get_kernel_config("CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS", NULL) == 
> IKCONFIG_Y)
> +                       efficient_unaligned_access = TRUE;
> +       }
>   
> 
> --
> Crash-utility mailing list -- devel@lists.crash-utility.osci.io
> To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
> https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
> Contribution Guidelines: https://github.com/crash-utility/crash/wiki
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

Reply via email to