All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	kernel@pengutronix.de, Andrew Morton <akpm@linux-foundation.org>,
	Gao Xiang <xiang@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 0/3] watchdog servicing during decompression
Date: Thu, 17 Oct 2019 13:48:15 +0100	[thread overview]
Message-ID: <20191017124815.GE25745@shell.armlinux.org.uk> (raw)
In-Reply-To: <c4b6805b-67fe-6bce-1777-2d81e96b4ac9@rasmusvillemoes.dk>

On Thu, Oct 17, 2019 at 02:34:52PM +0200, Rasmus Villemoes wrote:
> On 17/10/2019 14.03, Russell King - ARM Linux admin wrote:
> > We used to have this on ARM - it was called from the decompressor
> > code via an arch_decomp_wdog() hook.
> > 
> > That code got removed because it is entirely unsuitable for a multi-
> > platform kernel.  This looks like it takes an address for the watchdog
> > from the Kconfig, and builds that into the decompressor, making the
> > decompressor specific to that board or platform.
> > 
> > I'm not sure distros are going to like that given where we are with
> > multiplatform kernels.
> 
> This is definitely not for multiplatform kernels or general distros,
> it's for kernels that are built as part of a BSP for a specific board -
> hence the "Say N unless you know you need this.".
> 
> I didn't know it used to exist. But I do know that something like this
> is carried out-of-tree for lots of boards, so I thought I'd try to get
> upstream support.

Sorry, it does still exist, just been moved around a bit.

See lib/inflate.c:

STATIC int INIT inflate(void)
{
...
#ifdef ARCH_HAS_DECOMP_WDOG
    arch_decomp_wdog();
#endif

Given that it still exists, maybe this hook name should be used for
this same issue in the LZ4 code?

> The first two patches, or something like them, would be nice on their
> own, as that would minimize the conflicts when forward-porting the
> board-specific patch. But such a half-implemented feature that requires
> out-of-tree patches to actually do anything useful of course won't fly.
> 
> I'm not really a big fan of the third patch, even though it does work
> for all the cases I've encountered so far - I'm sure there would be
> boards where a much more complicated solution would be needed. Another
> method I thought of was to just supply a __weak no-op
> decompress_keepalive(), and then have a config option to point at an
> extra object file to link into the compressed/vmlinux (similar to the
> initramfs_source option that also points to some external resource).
> 
> But if the mainline kernel doesn't want anything like this
> re-introduced, that's also fine, that just means a bit of job security.

Well, we'll see whether the arm-soc people have anything to add...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Gao Xiang <xiang@kernel.org>,
	kernel@pengutronix.de, Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 0/3] watchdog servicing during decompression
Date: Thu, 17 Oct 2019 13:48:15 +0100	[thread overview]
Message-ID: <20191017124815.GE25745@shell.armlinux.org.uk> (raw)
In-Reply-To: <c4b6805b-67fe-6bce-1777-2d81e96b4ac9@rasmusvillemoes.dk>

On Thu, Oct 17, 2019 at 02:34:52PM +0200, Rasmus Villemoes wrote:
> On 17/10/2019 14.03, Russell King - ARM Linux admin wrote:
> > We used to have this on ARM - it was called from the decompressor
> > code via an arch_decomp_wdog() hook.
> > 
> > That code got removed because it is entirely unsuitable for a multi-
> > platform kernel.  This looks like it takes an address for the watchdog
> > from the Kconfig, and builds that into the decompressor, making the
> > decompressor specific to that board or platform.
> > 
> > I'm not sure distros are going to like that given where we are with
> > multiplatform kernels.
> 
> This is definitely not for multiplatform kernels or general distros,
> it's for kernels that are built as part of a BSP for a specific board -
> hence the "Say N unless you know you need this.".
> 
> I didn't know it used to exist. But I do know that something like this
> is carried out-of-tree for lots of boards, so I thought I'd try to get
> upstream support.

Sorry, it does still exist, just been moved around a bit.

See lib/inflate.c:

STATIC int INIT inflate(void)
{
...
#ifdef ARCH_HAS_DECOMP_WDOG
    arch_decomp_wdog();
#endif

Given that it still exists, maybe this hook name should be used for
this same issue in the LZ4 code?

> The first two patches, or something like them, would be nice on their
> own, as that would minimize the conflicts when forward-porting the
> board-specific patch. But such a half-implemented feature that requires
> out-of-tree patches to actually do anything useful of course won't fly.
> 
> I'm not really a big fan of the third patch, even though it does work
> for all the cases I've encountered so far - I'm sure there would be
> boards where a much more complicated solution would be needed. Another
> method I thought of was to just supply a __weak no-op
> decompress_keepalive(), and then have a config option to point at an
> extra object file to link into the compressed/vmlinux (similar to the
> initramfs_source option that also points to some external resource).
> 
> But if the mainline kernel doesn't want anything like this
> re-introduced, that's also fine, that just means a bit of job security.

Well, we'll see whether the arm-soc people have anything to add...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

  reply	other threads:[~2019-10-17 12:51 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-17 11:49 [RFC PATCH 0/3] watchdog servicing during decompression Rasmus Villemoes
2019-10-17 11:49 ` Rasmus Villemoes
2019-10-17 11:49 ` [RFC PATCH 1/3] decompress/keepalive.h: prepare for watchdog keepalive during kernel decompression Rasmus Villemoes
2019-10-17 11:49   ` Rasmus Villemoes
2019-10-17 11:49 ` [RFC PATCH 2/3] lib: lz4: wire up watchdog keepalive during decompression Rasmus Villemoes
2019-10-17 11:49   ` Rasmus Villemoes
2019-10-17 11:49 ` [RFC PATCH 3/3] decompress/keepalive.h: add config option for toggling a set of bits Rasmus Villemoes
2019-10-17 11:49   ` Rasmus Villemoes
2019-10-24 12:17   ` Linus Walleij
2019-10-24 12:17     ` Linus Walleij
2019-10-24 13:45     ` Rasmus Villemoes
2019-10-24 13:45       ` Rasmus Villemoes
2019-10-17 12:03 ` [RFC PATCH 0/3] watchdog servicing during decompression Russell King - ARM Linux admin
2019-10-17 12:03   ` Russell King - ARM Linux admin
2019-10-17 12:34   ` Rasmus Villemoes
2019-10-17 12:34     ` Rasmus Villemoes
2019-10-17 12:48     ` Russell King - ARM Linux admin [this message]
2019-10-17 12:48       ` Russell King - ARM Linux admin
2019-10-24 12:38     ` Linus Walleij
2019-10-24 12:38       ` Linus Walleij

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=20191017124815.GE25745@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=kernel@pengutronix.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=xiang@kernel.org \
    --cc=yamada.masahiro@socionext.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 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.