Received-SPF: pass (google.com: domain of linux-kernel+bounces-47634-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <8ce3c64a-344c-c2cb-7d36-4bd854a9ab7a@quicinc.com>
Date: Thu, 1 Feb 2024 10:20:06 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.1.0
Subject: Re: [PATCH v3 0/4] PM: hibernate: LZ4 compression support
To: "Rafael J. Wysocki" <rafael@kernel.org>
CC: Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
        "Jonathan
 Corbet" <corbet@lwn.net>,
        Peter Zijlstra <peterz@infradead.org>,
        "Paul E.
 McKenney" <paulmck@kernel.org>,
        "Steven Rostedt (Google)"
	<rostedt@goodmis.org>,
        Tejun Heo <tj@kernel.org>, Yan-Jie Wang
	<yanjiewtw@gmail.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        Catalin Marinas
	<catalin.marinas@arm.com>,
        <linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-pm@vger.kernel.org>, <quic_pkondeti@quicinc.com>,
        <quic_kprasan@quicinc.com>, <quic_mpilaniy@quicinc.com>,
        <quic_shrekk@quicinc.com>, <mpleshivenkov@google.com>,
        <ericyin@google.com>
References: <cover.1705927916.git.quic_nprakash@quicinc.com>
 <CAJZ5v0idcSPdwp7=WED0hYJ9s1KY_9w8Rkut8PCVGh56L0F5JA@mail.gmail.com>
Content-Language: en-US
From: Nikhil V <quic_nprakash@quicinc.com>
In-Reply-To: <CAJZ5v0idcSPdwp7=WED0hYJ9s1KY_9w8Rkut8PCVGh56L0F5JA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit


On 1/31/2024 7:50 PM, Rafael J. Wysocki wrote:
> On Mon, Jan 22, 2024 at 2:16 PM Nikhil V <quic_nprakash@quicinc.com> wrote:
>>
>> This patch series covers the following:
>> 1. Renaming lzo* to generic names, except for lzo_xxx() APIs. This is
>> used in the next patch where we move to crypto based APIs for
>> compression. There are no functional changes introduced by this
>> approach.
>>
>>
>> 2. Replace LZO library calls with crypto generic APIs
>>
>> Currently for hibernation, LZO is the only compression algorithm
>> available and uses the existing LZO library calls. However, there
>> is no flexibility to switch to other algorithms which provides better
>> results. The main idea is that different compression algorithms have
>> different characteristics and hibernation may benefit when it uses
>> alternate algorithms.
>>
>> By moving to crypto based APIs, it lays a foundation to use other
>> compression algorithms for hibernation.
>>
>>
>> 3. LZ4 compression
>>
>> Extend the support for LZ4 compression to be used with hibernation.
>> The main idea is that different compression algorithms have different
>> characteristics and hibernation may benefit when it uses any of these
>> algorithms: a default algorithm, having higher compression rate but is
>> slower(compression/decompression) and a secondary algorithm, that is
>> faster(compression/decompression) but has lower compression rate.
>>
>> LZ4 algorithm has better decompression speeds over LZO. This reduces
>> the hibernation image restore time.
>> As per test results:
>>                                      LZO             LZ4
>> Size before Compression(bytes)   682696704       682393600
>> Size after Compression(bytes)    146502402       155993547
>> Decompression Rate               335.02 MB/s     501.05 MB/s
>> Restore time                       4.4s             3.8s
>>
>> LZO is the default compression algorithm used for hibernation. Enable
>> CONFIG_HIBERNATION_DEF_COMP_LZ4 to set the default compressor as LZ4.
>>
>> Compression Benchmarks: https://github.com/lz4/lz4
>>
>>
>> 4. Support to select compression algorithm
>>
>> Currently the default compression algorithm is selected based on
>> Kconfig. Introduce a kernel command line parameter "hib_compression" to
>> override this behaviour.
>>
>> Users can set "hib_compression" command line parameter to specify
>> the algorithm.
>> Usage:
>>      LZO: hib_compression=lzo
>>      LZ4: hib_compression=lz4
>> LZO is the default compression algorithm used with hibernation.
>>
>>
>> Changes in v3:
>>   - Rebased to v6.8-rc1 after resolving the minor conflicts.
>>   - Link to v2:
>>   https://lore.kernel.org/all/cover.1700048610.git.quic_nprakash@quicinc.com/
> 
> I've applied the first 3 patches in the series (as 6.9 material), but
> I'm not particularly happy with the last one.
> 
> First off, I'm not sure if a kernel command line parameter is the most
> convenient way of selecting the compression algorithm.  Since (AFAICS)
> the restore kernel will detect the compression algo in use anyway (or
> at least it can be made do so), a modparam should work for this and it
> would be far more convenient to use.
> 
> Second, if I can be convinced that indeed, using a kernel command line
> option for this is the way to go, I don't particularly like the name
> used in that patch.
> 
> Please feel free to send a replacement for patch [4/4] separately.
> 
> Thanks!

Hi @Rafael,

Thanks for the update.

Regarding patch[4/4], will work on modifying the code as mentioned by 
adding module parameter instead of kernel cmdline parameter. Will send a 
separate patch for this after testing with the changes. Thanks for the 
input.

Thanks,
Nikhil V