All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lasse Collin <lasse.collin@tukaani.org>
To: Jonathan Corbet <corbet@lwn.net>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Sam James <sam@gentoo.org>,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
Subject: Re: [PATCH v2 08/16] docs: Add XZ_EXTERN to c_id_attributes
Date: Tue, 23 Jul 2024 20:54:37 +0300	[thread overview]
Message-ID: <20240723205437.3c0664b0@kaneli> (raw)
In-Reply-To: <87r0bms5da.fsf@trenco.lwn.net>

On 2024-07-21 Jonathan Corbet wrote:
> I spent a little while trying to figure out why we need XZ_EXTERN at
> all but lost in the #includes...

This is a good question. I looked at it and now I think that it's not
actually needed. Thus, this patch to Documentation/conf.py should be
dropped from this series, and I will submit a new patch to remove
XZ_EXTERN.

Preboot code on several archs has "#define STATIC static", for example:

    arch/x86/boot/compressed/misc.c
    arch/arm/boot/compressed/decompress.c
    arch/mips/boot/compressed/decompress.c
    drivers/firmware/efi/libstub/zboot.c

These files #include one of lib/decompress_*.c files. The STATIC macro
is used to detect if the code is being built for preboot code instead
of initramfs decompression. The STATIC macro is also used to make a few
functions static in lib/decompress_*.c files (and also in
lib/inflate.c).

Note that even if STATIC isn't initially defined, the
lib/decompress_*.c files have

    #include <linux/decompress/mm.h>

which will then "#define STATIC" (empty value).

lib/decompress_unxz.c makes all XZ functions static in preboot code via
the XZ_EXTERN macro. I'm not sure why I have done so. The commit
message from 2009 in my upstream tree isn't very specific.

STATIC is used also in lib/inflate.c to make functions static. However,
that file *seems* to be used only on alpha and nios2; it's *not* used
by lib/decompress_inflate.c which uses lib/zlib_inflate/inflate.c with
its extern functions in preboot code. But lib/inflate.c might have made
me think that there's a need to make functions static in some cases.

lib/decompress_unzstd.c is newer. It doesn't attempt to make all
functions static in preboot use.

Omitting XZ_EXTERN doesn't produce any warnings or make any difference
in x86 or ARM64 (CONFIG_EFI_ZBOOT=y) kernel sizes. (I'm ignoring a few
dozen bytes of noise on ARM64 between repeated builds.)

The boot code on PowerPC is special and it touches the XZ_EXTERN macro
in its xz_config.h. Relevant files:

    arch/powerpc/boot/xz_config.h
    arch/powerpc/boot/decompress.c

The "#undef XZ_EXTERN" can be confusing but in the end all XZ_EXTERN
uses become "static" still. Comparing to zlib usage on PowerPC, it
seems that decompressor functions aren't required to be static (zlib's
files are pre-processed with a sed script but it doesn't make anything
static). So, even without testing, it seems quite clear that removing
XZ_EXTERN would be fine on PowerPC too.

Thus, let's drop this patch to Documentation/conf.py, and I submit a
patch to remove XZ_EXTERN.

Thanks!

-- 
Lasse Collin

  reply	other threads:[~2024-07-23 17:54 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-21 13:36 [PATCH v2 00/16] xz: Updates to license, filters, and compression options Lasse Collin
2024-07-21 13:36 ` [PATCH v2 01/16] MAINTAINERS: Add XZ Embedded maintainer Lasse Collin
2024-07-21 13:36 ` [PATCH v2 02/16] LICENSES: Add 0BSD license text Lasse Collin
2024-07-21 13:36 ` [PATCH v2 03/16] xz: Switch from public domain to BSD Zero Clause License (0BSD) Lasse Collin
2024-07-21 13:36 ` [PATCH v2 04/16] xz: Fix comments and coding style Lasse Collin
2024-07-21 13:36 ` [PATCH v2 05/16] xz: Fix kernel-doc formatting errors in xz.h Lasse Collin
2024-07-22  2:38   ` Randy Dunlap
2024-07-21 13:36 ` [PATCH v2 06/16] xz: Improve the MicroLZMA kernel-doc " Lasse Collin
2024-07-21 13:36 ` [PATCH v2 07/16] xz: Documentation/staging/xz.rst: Revise thoroughly Lasse Collin
2024-07-22  3:19   ` Randy Dunlap
2024-07-21 13:36 ` [PATCH v2 08/16] docs: Add XZ_EXTERN to c_id_attributes Lasse Collin
2024-07-21 23:16   ` Jonathan Corbet
2024-07-23 17:54     ` Lasse Collin [this message]
2024-07-21 13:36 ` [PATCH v2 09/16] xz: Cleanup CRC32 edits from 2018 Lasse Collin
2024-07-22  4:29   ` Michael Ellerman
2024-07-21 13:36 ` [PATCH v2 10/16] xz: Optimize for-loop conditions in the BCJ decoders Lasse Collin
2024-07-21 13:36 ` [PATCH v2 11/16] xz: Add ARM64 BCJ filter Lasse Collin
2024-07-21 13:36 ` [PATCH v2 12/16] xz: Add RISC-V " Lasse Collin
2024-07-21 13:36 ` [PATCH v2 13/16] xz: Use 128 MiB dictionary and force single-threaded mode Lasse Collin
2024-07-21 13:36 ` [PATCH v2 14/16] xz: Adjust arch-specific options for better kernel compression Lasse Collin
2024-07-21 13:36   ` Lasse Collin
2024-07-21 13:36 ` [PATCH v2 15/16] arm64: boot: add Image.xz support Lasse Collin
2024-07-23 12:47   ` Simon Glass
2024-07-21 13:36 ` [PATCH v2 16/16] riscv: " Lasse Collin
2024-07-21 13:36   ` Lasse Collin
2024-07-21 16:01   ` Emil Renner Berthing
2024-07-21 16:01     ` Emil Renner Berthing
2024-07-24 11:05 ` [PATCH v2 17/16] xz: Remove XZ_EXTERN and extern from functions Lasse Collin
2024-07-24 11:05   ` Lasse Collin
2024-07-24 12:44   ` Michael Ellerman
2024-07-24 12:44     ` Michael Ellerman

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=20240723205437.3c0664b0@kaneli \
    --to=lasse.collin@tukaani.org \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sam@gentoo.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.