mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Bradley Morgan <bradley.morgan@linux.dev>
To: pmenzel@molgen.mpg.de
Cc: atomlin@atomlin.com, da.gomez@kernel.org, dsterba@suse.com,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-modules@vger.kernel.org, mcgrof@kernel.org,
	nathan@kernel.org, nsc@kernel.org, petr.pavlu@suse.com,
	samitolvanen@google.com, terrelln@fb.com
Subject: Re: [PATCH v2] module: Add LZ4 module compression
Date: Sun, 15 Nov 2026 17:35:32 +0000	[thread overview]
Message-ID: <F2B70664-1E9A-427C-9132-A0F5F586F731@linux.dev> (raw)
In-Reply-To: <20261115120416.256087-1-pmenzel@molgen.mpg.de>

On 15 November 2026 12:04:13 GMT, Paul Menzel <pmenzel@molgen.mpg.de>
wrote:
>XZ modules are small but slow to decompress in the kernel. Therefore,
>support modules compressed in the LZ4 frame format (.ko.lz4), as written
>by lz4(1). Decode block by block into a scratch buffer and copy into the
>module pages. Linked blocks keep the last 64 KiB of output in front of
>the scratch buffer as the dictionary. Verify the header checksum, block
>checksums, content checksum and content size when the frame has them.
>Frames with a dictionary ID are rejected.

.ko.lz4? when would this be used..?

>
>modules_install uses `lz4 -12 -B4 -BD`: 64 KiB linked blocks are as
>small as 4 MiB blocks (i915.ko: 3587140 vs 3585364 bytes) and keep the
>scratch buffer at 128 KiB.
>
>Userspace needs kmod with LZ4 support for depmod and modprobe [1].
>
>On a Dell XPS 13 9310 with an Intel Core i7-1165G7 (Tiger Lake) and an
>NVMe SSD, decompressing and checking the signatures of the 237 modules
>loaded during boot takes 624 ms of CPU time with XZ (i915: 91 ms) and
>131 ms with LZ4 (i915: 12 ms), measured by calling finit_module() again
>for each loaded module. The LZ4 modules take 20.3 MB instead of 11.0 MB.
>Both kernels are v7.4-rc1 with the same patches, including pending
>lib/xz decoder speed-ups. Ten alternating boots of each kernel:
>
>  | ms (median)                               |  XZ  | LZ4  | change |
>  +-------------------------------------------+------+------+--------+
>  | early-i915 (modprobe i915 + dependencies) |  295 |   90 |   -205 |
>  | first i915 message                        |  738 |  547 |   -192 |
>  | GDM autologin                             |  973 |  876 |    -96 |
>  | GRUB start to autologin                   | 1292 | 1197 |    -94 |
>
>QEMU 11.1.2 (KVM, AMD EPYC 7763, 8 vCPUs, module.sig_enforce=1, 10
>rounds), loading the same 237 modules in dependency order:
>
>  | median                   |   XZ    |   LZ4   | uncompressed |
>  +--------------------------+---------+---------+--------------+
>  | one at a time            | 2082 ms | 1499 ms |      1461 ms |
>  | 8 in parallel, like udev |  619 ms |  573 ms |       593 ms |

This table doesn't look very nicely formatted for me.

>
>Link: https://github.com/kmod-project/kmod/pull/505 [1]
>Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>---
>
>Notes:
>    Changes in v2:
>    - Declare retval as ssize_t, so output of 2 GiB or more is not
>      truncated (Sashiko). The zstd decoder has the same bug, fixed by
>      the separate patch "module/decompress: Use ssize_t for the zstd
>      decompressed size".
>    - Reject empty frames with -EINVAL; v1 failed vmap() of 0 pages with
>      -ENOMEM (Sashiko).
>    - Describe the decoder and link the frame format specification, group
>      the definitions by FLG byte, BD byte and block size field, compare the
>      version with FIELD_GET(), rename LZ4F_DICT_SIZE to LZ4F_WINDOW_SIZE,
>      look up the block maximum size in a table, shorten the dictionary
>      comment (Bradley Morgan).
>    
>    v1: https://lore.kernel.org/all/20261113144233.193782-1-pmenzel@molgen.mpg.de/
>    
>    Claude Opus 5.5 (Claude Code) wrote the decoder, the v2
>    changes and this message. Tests: W=1 compile (GCC 16.2, Debian config
>    for x86_64, tinyconfig for i386); QEMU with module.sig_enforce=1: 11 LZ4
>    frame variants (64 KiB-4 MiB, linked/independent, block/content
>    checksums, content size, stored blocks) load, 9 corrupt, truncated or
>    empty files are rejected with EINVAL or EKEYREJECTED (v1: ENOMEM for the
>    empty ones); the 237-module boot set loads in QEMU. v1 also loaded on a
>    Dell XPS 13 9310 (20 boots); v2 was not booted on hardware.

Could you do a favour and test patches you submit, just in case of the
edgecases :)

>
> kernel/module/Kconfig      |  13 +-
> kernel/module/decompress.c | 243 +++++++++++++++++++++++++++++++++++++
> scripts/Makefile.modinst   |   6 +
> 3 files changed, 261 insertions(+), 1 deletion(-)
>
>diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
>index 43b1bb01fd273..36f12612c74dd 100644
>--- a/kernel/module/Kconfig
>+++ b/kernel/module/Kconfig
>@@ -344,7 +344,7 @@ config MODULE_COMPRESS
> 	  This is fully compatible with signed modules.
> 
> 	  The tool used to work with modules needs to support the selected
>-	  compression type. kmod MAY support gzip, xz and zstd. Other tools
>+	  compression type. kmod MAY support gzip, xz, zstd and lz4. Other tools
> 	  might have a limited selection of the supported types.
> 
> 	  Note that for modules inside an initrd or initramfs, it's more
>@@ -377,6 +377,15 @@ config MODULE_COMPRESS_ZSTD
> 	  Support modules compressed with ZSTD. The installed modules are
> 	  suffixed with .ko.zst.
> 
>+config MODULE_COMPRESS_LZ4
>+	bool "LZ4"
>+	help
>+	  Support modules compressed with LZ4. The installed modules are
>+	  suffixed with .ko.lz4.
>+
>+	  LZ4 modules are larger than XZ or ZSTD ones but decompress
>+	  several times faster.
>+
> endchoice
> 
> config MODULE_COMPRESS_ALL
>@@ -394,6 +403,8 @@ config MODULE_DECOMPRESS
> 	select ZLIB_INFLATE if MODULE_COMPRESS_GZIP
> 	select XZ_DEC if MODULE_COMPRESS_XZ
> 	select ZSTD_DECOMPRESS if MODULE_COMPRESS_ZSTD
>+	select LZ4_DECOMPRESS if MODULE_COMPRESS_LZ4
>+	select XXHASH if MODULE_COMPRESS_LZ4
> 	help
> 	  Support for decompressing kernel modules by the kernel itself
> 	  instead of relying on userspace to perform this task. Useful when
>diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c
>index cce098671be97..028b411015f21 100644
>--- a/kernel/module/decompress.c
>+++ b/kernel/module/decompress.c
>@@ -287,6 +287,249 @@ static ssize_t module_zstd_decompress(struct load_info *info,
> 	kvfree(wksp);
> 	return retval;
> }
>+#elif defined(CONFIG_MODULE_COMPRESS_LZ4)
>+#include <linux/bitfield.h>
>+#include <linux/lz4.h>
>+#include <linux/sizes.h>
>+#include <linux/unaligned.h>
>+#include <linux/xxhash.h>
>+#define MODULE_COMPRESSION	lz4
>+#define MODULE_DECOMPRESS_FN	module_lz4_decompress
>+

Oh these includes mid file are a MESS! No, please no. Good to if defined,
but I'm sure the maintainer wouldn't mind if you ifdefed those includes on
the TOP of the file. Like any other normal kernel file.

>+/*
>+ * Decoder for the LZ4 frame format written by lz4(1), see
>+ * https://github.com/lz4/lz4/blob/dev/doc/lz4_Frame_format.md
>+ *
>+ * Each block is decoded into a scratch buffer and copied into the module
>+ * pages. A linked block may reference up to 64 KiB of the output before
>+ * it, so that much output is kept in front of the scratch buffer.
>+ */

Well, I suggested something better,

/* Decoder for the lz4 frame format,
 * written by lz4 [1]
 * This code adds support for lz4
 * modules inside the kernel, so 
 * that lz4 modules can be used.
 * 
 * [1] - (your link, lz4 frame format src??)
 */

What about this?

I say bikeshed, but let's not make it TOO sloppy, you reckon?

>+#define LZ4F_MAGIC		0x184d2204
>+
>+/* Frame descriptor, FLG byte */
>+#define LZ4F_FLG_VERSION	GENMASK(7, 6)
>+#define LZ4F_FLG_BLOCK_INDEP	BIT(5)
>+#define LZ4F_FLG_BLOCK_CSUM	BIT(4)
>+#define LZ4F_FLG_CONTENT_SIZE	BIT(3)
>+#define LZ4F_FLG_CONTENT_CSUM	BIT(2)
>+#define LZ4F_FLG_RESERVED	BIT(1)
>+#define LZ4F_FLG_DICT_ID	BIT(0)
>+
>+/* Frame descriptor, BD byte */
>+#define LZ4F_BD_BLOCK_MAXSIZE	GENMASK(6, 4)
>+#define LZ4F_BD_RESERVED	(BIT(7) | GENMASK(3, 0))
>+
>+/* Block size field: the top bit marks a block stored uncompressed */
>+#define LZ4F_BLOCK_UNCOMPRESSED	BIT(31)
>+
>+#define LZ4F_WINDOW_SIZE	SZ_64K
>+
>+/* Block maximum sizes for the BD byte's Block MaxSize values 4 to 7 */
>+static const u32 lz4f_block_max[] = { SZ_64K, SZ_256K, SZ_1M, SZ_4M };
>+
>+static int module_lz4_copy_out(struct load_info *info, const u8 *src,
>+			       size_t len, size_t *total)
>+{
>+	while (len) {
>+		size_t offset = *total % PAGE_SIZE;
>+		size_t chunk = min_t(size_t, len, PAGE_SIZE - offset);
>+		struct page *page;
>+
>+		if (offset)
>+			page = info->pages[info->used_pages - 1];
>+		else
>+			page = module_get_next_page(info);
>+		if (IS_ERR(page))
>+			return PTR_ERR(page);
>+
>+		memcpy_to_page(page, offset, src, chunk);
>+		src += chunk;
>+		len -= chunk;
>+		*total += chunk;
>+	}
>+
>+	return 0;
>+}
>+
>+static int module_lz4_verify_content(struct load_info *info, size_t total,
>+				     u32 expected)
>+{
>+	void *data;
>+	u32 csum;
>+
>+	data = vmap(info->pages, info->used_pages, VM_MAP, PAGE_KERNEL_RO);
>+	if (!data)
>+		return -ENOMEM;
>+	csum = xxh32(data, total, 0);
>+	vunmap(data);
>+
>+	if (csum != expected) {
>+		pr_err("LZ4-compressed data is corrupt (content checksum)\n");
>+		return -EINVAL;
>+	}
>+
>+	return 0;
>+}
>+
>+static ssize_t module_lz4_decompress(struct load_info *info,
>+				     const void *buf, size_t size)
>+{
>+	const u8 *in = buf, *end = in + size, *desc;
>+	size_t block_max, total = 0, dict_len = 0;
>+	u64 content_size = 0;
>+	u8 *scratch, *out;
>+	bool linked;
>+	u8 flg, bd;
>+	ssize_t retval;
>+
>+	if (size < 7 || get_unaligned_le32(in) != LZ4F_MAGIC) {
>+		pr_err("not an lz4 compressed module\n");
>+		return -EINVAL;
>+	}
>+
>+	desc = in + 4;
>+	flg = desc[0];
>+	bd = desc[1];
>+	in = desc + 2;
>+	if (FIELD_GET(LZ4F_FLG_VERSION, flg) != 1 ||
>+	    (flg & LZ4F_FLG_RESERVED) || (bd & LZ4F_BD_RESERVED) ||
>+	    FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) < 4) {
>+		pr_err("LZ4-compressed data has an unsupported frame descriptor\n");

When would this be reached?

>+		return -EINVAL;
>+	}
>+	if (flg & LZ4F_FLG_DICT_ID) {
>+		pr_err("LZ4-compressed data needs a dictionary\n");
>+		return -EINVAL;
>+	}
>+	if (flg & LZ4F_FLG_CONTENT_SIZE) {
>+		if (end - in < 8)
>+			goto truncated_header;
>+		content_size = get_unaligned_le64(in);
>+		in += 8;
>+	}
>+	if (end - in < 1)
>+		goto truncated_header;
>+	if (*in != ((xxh32(desc, in - desc, 0) >> 8) & 0xff)) {
>+		pr_err("LZ4-compressed data has a corrupt frame descriptor\n");

Is there a mechanism in the future in order to understand the garbled crap
and fix it to make it manageable? (This can be future patch)

>+		return -EINVAL;
>+	}
>+	in++;
>+
>+	block_max = lz4f_block_max[FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) - 4];
>+	linked = !(flg & LZ4F_FLG_BLOCK_INDEP);
>+
>+	scratch = kvmalloc((linked ? LZ4F_WINDOW_SIZE : 0) + block_max,
>+			   GFP_KERNEL);
>+	if (!scratch)
>+		return -ENOMEM;
>+	out = scratch + (linked ? LZ4F_WINDOW_SIZE : 0);
>+
>+	for (;;) {
>+		u32 block_size;
>+		size_t len;
>+		int ret;
>+
>+		if (end - in < 4)
>+			goto truncated;
>+		block_size = get_unaligned_le32(in);
>+		in += 4;
>+		if (!block_size)
>+			break;
>+
>+		len = block_size & ~LZ4F_BLOCK_UNCOMPRESSED;
>+		if (len > block_max) {
>+			pr_err("LZ4-compressed data has an oversized block\n");
>+			retval = -EINVAL;
>+			goto out;
>+		}
>+		if ((size_t)(end - in) < len +
>+		    (flg & LZ4F_FLG_BLOCK_CSUM ? 4 : 0))
>+			goto truncated;
>+
>+		if ((flg & LZ4F_FLG_BLOCK_CSUM) &&
>+		    xxh32(in, len, 0) != get_unaligned_le32(in + len)) {
>+			pr_err("LZ4-compressed data is corrupt (block checksum)\n");
>+			retval = -EINVAL;
>+			goto out;
>+		}
>+
>+		if (block_size & LZ4F_BLOCK_UNCOMPRESSED) {
>+			memcpy(out, in, len);
>+			ret = len;
>+		} else if (linked) {
>+			ret = LZ4_decompress_safe_usingDict(in, out, len,
>+							    block_max,
>+							    out - dict_len,
>+							    dict_len);
>+		} else {
>+			ret = LZ4_decompress_safe(in, out, len, block_max);
>+		}
>+		if (ret < 0) {
>+			pr_err("LZ4-decompression failed\n");
>+			retval = -EINVAL;
>+			goto out;
>+		}
>+
>+		retval = module_lz4_copy_out(info, out, ret, &total);
>+		if (retval)
>+			goto out;
>+
>+		if (linked) {
>+			/* Keep the last 64 KiB as the next block's dictionary. */
>+			size_t keep = min_t(size_t, dict_len + ret,
>+					    LZ4F_WINDOW_SIZE);
>+
>+			memmove(out - keep, out + ret - keep, keep);
>+			dict_len = keep;
>+		}
>+
>+		in += len;
>+		if (flg & LZ4F_FLG_BLOCK_CSUM)
>+			in += 4;
>+	}
>+
>+	if (!total) {
>+		pr_err("LZ4-compressed module is empty\n");
>+		retval = -EINVAL;
>+		goto out;
>+	}
>+
>+	if (flg & LZ4F_FLG_CONTENT_CSUM) {
>+		if (end - in < 4)
>+			goto truncated;
>+		retval = module_lz4_verify_content(info, total,
>+						   get_unaligned_le32(in));
>+		if (retval)
>+			goto out;
>+		in += 4;
>+	}
>+
>+	if (in != end) {
>+		pr_err("LZ4-compressed module has trailing data\n");
>+		retval = -EINVAL;
>+		goto out;
>+	}
>+
>+	if ((flg & LZ4F_FLG_CONTENT_SIZE) && content_size != total) {
>+		pr_err("LZ4-compressed data has a wrong content size\n");
>+		retval = -EINVAL;
>+		goto out;
>+	}
>+
>+	retval = total;
>+	goto out;
>+
>+truncated:
>+	pr_err("LZ4-compressed data is truncated\n");
>+	retval = -EINVAL;
>+out:
>+	kvfree(scratch);
>+	return retval;
>+
>+truncated_header:
>+	pr_err("LZ4-compressed data is truncated\n");
>+	return -EINVAL;
>+}
> #else
> #error "Unexpected configuration for CONFIG_MODULE_DECOMPRESS"
> #endif

My concern here is that some of these gnarly functions are uncommented, so
when a noob reads this code, "Oh no! What does this do! Oops... I broke my
module"


Maybe comment for functions that are gnarly and you think a reader would
need to understand to be able to operate/contribute.

>diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
>index 9ba45e5b32b18..aebc5e89d0e75 100644
>--- a/scripts/Makefile.modinst
>+++ b/scripts/Makefile.modinst
>@@ -57,6 +57,7 @@ ifdef CONFIG_MODULE_COMPRESS_ALL
> suffix-$(CONFIG_MODULE_COMPRESS_GZIP)	:= .gz
> suffix-$(CONFIG_MODULE_COMPRESS_XZ)	:= .xz
> suffix-$(CONFIG_MODULE_COMPRESS_ZSTD)	:= .zst
>+suffix-$(CONFIG_MODULE_COMPRESS_LZ4)	:= .lz4
> endif
> 
> modules := $(patsubst %.o, $(dst)/%.ko$(suffix-y), $(modules))
>@@ -151,6 +152,8 @@ quiet_cmd_xz = XZ      $@
>       cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
> quiet_cmd_zstd = ZSTD    $@
>       cmd_zstd = $(ZSTD) --rm -f -q $<
>+quiet_cmd_lz4 = LZ4     $@
>+      cmd_lz4 = $(LZ4) -12 -B4 -BD --rm -f -q $< $@
> 
> $(dst)/%.ko.gz: $(dst)/%.ko FORCE
> 	$(call cmd,gzip)
>@@ -161,6 +164,9 @@ $(dst)/%.ko.xz: $(dst)/%.ko FORCE
> $(dst)/%.ko.zst: $(dst)/%.ko FORCE
> 	$(call cmd,zstd)
> 
>+$(dst)/%.ko.lz4: $(dst)/%.ko FORCE
>+	$(call cmd,lz4)
>+
> PHONY += FORCE
> FORCE:
> 
>

--- Thanks!

  reply	other threads:[~2026-11-15 17:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-15 12:04 Paul Menzel
2026-11-15 17:35 ` Bradley Morgan [this message]
2026-11-15 21:09   ` Paul Menzel
2026-11-15 21:16     ` Bradley Morgan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F2B70664-1E9A-427C-9132-A0F5F586F731@linux.dev \
    --to=bradley.morgan@linux.dev \
    --cc=atomlin@atomlin.com \
    --cc=da.gomez@kernel.org \
    --cc=dsterba@suse.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=nathan@kernel.org \
    --cc=nsc@kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=pmenzel@molgen.mpg.de \
    --cc=samitolvanen@google.com \
    --cc=terrelln@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®