[Crash-utility] Re: 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/01 17:48, Tao Liu wrote:
> Hi Yulong,
> 
> On Fri, Mar 1, 2024 at 4:25 PM tang yulong <1637403063@xxxxxx> wrote:
>>
>>> Hi Yulong,
>>>
>>> Thanks for your patch!
>>>
>>> On Mon, Feb 26, 2024 at 3:20 PM Yulong TANG 汤玉龙 <yulong.tang(a)nio.com&gt; wrote:
>>>
>>> I have no objection to the lzo-rle decompression feature for crash.
>>> However I have some concern of your patch:
>>>
>>> The patch you attached is a "lzorle_decompress_safe" implementation
>>> which is copied from kernel source code. One of the drawbacks of
>>> copying kernel source code is, kernel is constantly evolving, the code
>>> you copied here today maybe updated someday later, and in support of
>>> different kernel versions, we need to keep a bunch of
>>> switch(kernel_version) and case code to keep the compatibility, which
>>> is what we are trying to avoid.
>>>
>>> In addition, the code you copied has deliberately deleted the "if
>>> defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)" part, which may also
>>> cause some problem, and as far as I know, there is no good way in
>>> crash to determine the kernel config status, please feel free to
>>> correct me if I'm wrong.
>>
>> Hi Tao,
>>
>> I discovered that there seems to be an array called ikconfig_all in kernel.c which records the kernel config status, and there is an API called get_kernel_config(char *conf_name, char **str) for obtaining kernel configuration. This might help solve the second problem you raised.
>> For example, adding a line:
>> #define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS get_kernel_config("CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS", NULL);
>>
> 
> OK, thanks for the clarification, sounds workable. Then what do you
> think if we keep the "if
> defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)" related code of the
> copied kernel code, but change the "if defined" macro into a runtime
> check like "if get_kernel_config("CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS",
> NULL)) == IKCONFIG_Y". So the code can be chosen whether to execute
> depending on the config status.

hmm, as far as I know, not many kernels are configured with 
CONFIG_IKCONFIG=y, e.g. RHEL kernels don't have it.  so if it depends 
only on ikconfig, it will not work on many kernels.

It's good to use ikconfig, but isn't there any other way?

For example, it looks like it's always set to y on x86.  so maybe we can 
have a default value depending on architecture, for no ikconfig kernels.

config X86
...
         select HAVE_EFFICIENT_UNALIGNED_ACCESS

Thanks,
Kazu
--
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