Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Nick Terrell <terrelln@fb.com>
To:     Oleksandr Natalenko <oleksandr@natalenko.name>
CC:     Piotr Gorski <lucjan.lucjanov@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Michal Marek <michal.lkml@markovi.net>,
        Andrew Morton <akpm@linux-foundation.org>,
        Kees Cook <keescook@chromium.org>,
        "Nathan Chancellor" <nathan@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Valentin Schneider <valentin.schneider@arm.com>,
        "Jan Alexander Steffens (heftig)" <jan.steffens@gmail.com>,
        David Howells <dhowells@redhat.com>,
        Johannes Weiner <hannes@cmpxchg.org>
Subject: Re: [PATCH] init: add support for zstd compressed modules
Thread-Topic: [PATCH] init: add support for zstd compressed modules
Thread-Index: AQHXJVrjTl/b3y00YEOpOt7F0WiTWaqeXt4A
Date:   Wed, 31 Mar 2021 17:39:25 +0000
Message-ID: <33D7F6C9-6F53-471A-A146-7F128F6918F5@fb.com>
References: <20210330113235.2767216-1-lucjan.lucjanov@gmail.com>
 <20210330115023.qt742qsdekwiroey@spock.localdomain>
In-Reply-To: <20210330115023.qt742qsdekwiroey@spock.localdomain>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <27848AA768DEEE439474A8D41AD6DE47@namprd15.prod.outlook.com>
X-MS-Exchange-CrossTenant-Network-Message-Id: e1313b89-550e-4aec-e3eb-08d8f46bed92
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Mar 2021 17:39:25.5091
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Mwf5eJcsf+Y96HMgLg7co9lOHgblhJCz1jicGWkVQoogPjp1dp38R8bCVbk5UgU3
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR15MB3618
X-OriginatorOrg: fb.com
X-Proofpoint-GUID: psKsCM9V9n1uzG1jYC_uESzSNpMHMvHN
X-Proofpoint-ORIG-GUID: psKsCM9V9n1uzG1jYC_uESzSNpMHMvHN
Content-Transfer-Encoding: quoted-printable
X-Proofpoint-UnRewURL: 0 URL was un-rewritten
MIME-Version: 1.0
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.369,18.0.761
 definitions=2021-03-31_08:2021-03-31,2021-03-31 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 malwarescore=0
 suspectscore=0 bulkscore=0 adultscore=0 impostorscore=0 spamscore=0
 mlxlogscore=999 clxscore=1011 priorityscore=1501 lowpriorityscore=0
 mlxscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2103300000 definitions=main-2103310122
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Mar 30, 2021, at 4:50 AM, Oleksandr Natalenko <oleksandr@natalenko.nam=
e> wrote:
>=20
> On Tue, Mar 30, 2021 at 01:32:35PM +0200, Piotr Gorski wrote:
>> kmod 28 supports modules compressed in zstd format so let's add this pos=
sibility to kernel.
>>=20
>> Signed-off-by: Piotr Gorski <lucjan.lucjanov@gmail.com>
>> ---
>> Makefile     | 7 +++++--
>> init/Kconfig | 9 ++++++---
>> 2 files changed, 11 insertions(+), 5 deletions(-)
>>=20
>> diff --git a/Makefile b/Makefile
>> index 5160ff8903c1..82f4f4cc2955 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1156,8 +1156,8 @@ endif # INSTALL_MOD_STRIP
>> export mod_strip_cmd
>>=20
>> # CONFIG_MODULE_COMPRESS, if defined, will cause module to be compressed
>> -# after they are installed in agreement with CONFIG_MODULE_COMPRESS_GZIP
>> -# or CONFIG_MODULE_COMPRESS_XZ.
>> +# after they are installed in agreement with CONFIG_MODULE_COMPRESS_GZI=
P,
>> +# CONFIG_MODULE_COMPRESS_XZ, or CONFIG_MODULE_COMPRESS_ZSTD.
>>=20
>> mod_compress_cmd =3D true
>> ifdef CONFIG_MODULE_COMPRESS
>> @@ -1167,6 +1167,9 @@ ifdef CONFIG_MODULE_COMPRESS
>>   ifdef CONFIG_MODULE_COMPRESS_XZ
>>     mod_compress_cmd =3D $(XZ) --lzma2=3Ddict=3D2MiB -f
>>   endif # CONFIG_MODULE_COMPRESS_XZ
>> +  ifdef CONFIG_MODULE_COMPRESS_ZSTD
>> +    mod_compress_cmd =3D $(ZSTD) -T0 --rm -f -q

This will use the default zstd level, level 3. I think it would make more s=
ense to use a high
compression level. Level 19 would probably be a good choice. That will choo=
se a window
size of up to 8MB, meaning the decompressor needs to allocate that much mem=
ory. If that
is unacceptable, you could use `zstd -T0 --rm -f -q -19 --zstd=3Dwlog=3D21`=
, which will use a
window size of up to 2MB, to match the XZ command. Note that if the file is=
 smaller than
the window size, it will be shrunk to the smallest power of two at least as=
 large as the file.

Best,
Nick Terrell

>> +  endif # CONFIG_MODULE_COMPRESS_ZSTD
>> endif # CONFIG_MODULE_COMPRESS
>> export mod_compress_cmd
>>=20
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 8c2cfd88f6ef..86a452bc2747 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -2250,8 +2250,8 @@ config MODULE_COMPRESS
>> 	bool "Compress modules on installation"
>> 	help
>>=20
>> -	  Compresses kernel modules when 'make modules_install' is run; gzip or
>> -	  xz depending on "Compression algorithm" below.
>> +	  Compresses kernel modules when 'make modules_install' is run; gzip,
>> +	  xz, or zstd depending on "Compression algorithm" below.
>>=20
>> 	  module-init-tools MAY support gzip, and kmod MAY support gzip and xz.
>>=20
>> @@ -2273,7 +2273,7 @@ choice
>> 	  This determines which sort of compression will be used during
>> 	  'make modules_install'.
>>=20
>> -	  GZIP (default) and XZ are supported.
>> +	  GZIP (default), XZ, and ZSTD are supported.
>>=20
>> config MODULE_COMPRESS_GZIP
>> 	bool "GZIP"
>> @@ -2281,6 +2281,9 @@ config MODULE_COMPRESS_GZIP
>> config MODULE_COMPRESS_XZ
>> 	bool "XZ"
>>=20
>> +config MODULE_COMPRESS_ZSTD
>> +	bool "ZSTD"
>> +
>> endchoice
>>=20
>> config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
>> --=20
>> 2.31.0.97.g1424303384
>>=20
>=20
> Great!
>=20
> Reviewed-by: Oleksandr Natalenko <oleksandr@natalenko.name>
>=20
> This works perfectly fine in Arch Linux if accompanied by the
> following mkinitcpio amendment: [1].
>=20
> I'm also Cc'ing other people from get_maintainers output just
> to make this submission more visible.
>=20
> Thanks.
>=20
> [1] https://github.com/archlinux/mkinitcpio/pull/43
>=20
> --=20
>  Oleksandr Natalenko (post-factum)