[Crash-utility] Re: [PATCH v3 ] Adding the zram decompression algorithm "lzo-rle" to support kernel versions >= 5.1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2024/03/28 15:38, HAGIO KAZUHITO(萩尾 一仁) wrote:
> 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.

I found a fatal error during "make extensions":

$ make extensions
gcc -Wall -g -shared -rdynamic -o echo.so echo.c -fPIC -DX86_64 -DLZO 
-DSNAPPY -DZSTD -DGDB_10_2
In file included from echo.c:18:
defs.h:54:10: fatal error: lzorle_decompress.h: No such file or directory
  #include "lzorle_decompress.h"
           ^~~~~~~~~~~~~~~~~~~~~
compilation terminated.
...

--- a/defs.h
+++ b/defs.h
@@ -51,6 +51,7 @@
  #include <regex.h>
  #ifdef LZO
  #include <lzo/lzo1x.h>
+#include "lzorle_decompress.h"
  #endif
  #ifdef SNAPPY
  #include <snappy-c.h>

we should move this to diskdump.c ?

Thanks,
Kazu

> 
> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxx
>> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
>> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxx
> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux