Re: [PATCH 0/2] Make squashfs fragments' cache size more configurable

[Phillip Lougher]

On Thu, Oct 19, 2017 at 12:50 AM, Qixuan Wu <wuqixuan@xxxxxxxxxx> wrote:
> Hi All,
>
> Currently, squashfs fragments' cache size is only determined by
> config option CONFIG_SQUASHFS_FRAGMENT_CACHE_SIZE. Users have
> no way to change the value when they get the binary kernel.

Thank-you for the patches, but they're both pointless and dangerous.
Let's be clear here you're trying to change an "expert only" kernel
configuration option into a user changeable option.  This is stupid
because it is not meant for non-experts to change for good reason.

The fragment cache size isn't  some small tweak to the operation of
Squashfs, it fundamentally affects both the performance and memory
overhead of Squashfs.  As such right from its introduction in 2003, it
has been an "expert only" configuration option at build time.  Even
then it is made clear that the default has been carefully chosen, and
it should only be changed in exceptional circumstances.  This
basically means don't change the default unless you really know what
you're doing, and this means tracing of Squashfs against your use-case
to determine caching behaviour.  There is absolutely no other reason
why you'd want to change the default.  This also means it should be
restricted to kernel configuration time only.

Let's be clear again, very few people should ever want to change the
default, and for the "experts" that do want to do so, they can do so
when configuring the kernel.  If you're not in a position to change it
at kernel configuration time then by definition you're not an expert,
and you shouldn't be able to change it anyway and certainly not as a
user.

There is absolutely no use-case here to make this a user changeable
option.  I can see no upsides in doing this, only downsides.

Frankly if you need to change this value at module insert time then
there is something wrong with your system or build process.   If you
want this because you want to build the kernel/modules once, and then
post-facto configure them for various products then it is your build
process that is broken.   If you want this because you want to
dynamically change Squashfs memory usage/caching behaviour post kernel
configuration time it suggests you're trying to adapt Squashfs's
footprint based on available memory.   This is an abuse of the option
as it's only meant to be used after detailed tracing/analysis and
certainly not used to accommodate unforeseen dynamic low memory
situations, and if that's the reason for needing this option, you
should be looking to solve it elsewhere.

Ultimately this has been an "expert"  kernel configuration only option
since its introduction in 2003, and I never been asked to change it,
and I believe this is because people recognise it as such.  I suspect
you're trying to change this for fundamentally bogus reasons.
Moreover Squashfs is used in many different use-cases and
distributions, and I'm not going to make this a user-changeable option
allowing users to insert the Squashfs module in such a way that will
break its performance.

So NACK.

Phillip Lougher (Squashfs maintainer)

> Now make it be configured when booting or inserting module.
> Actually, it's better that a config option in a number format
> in .config file cat be reconfigured during booting or inserting
> module.
>
> Thanks
> Qixuan
>
> Qixuan Wu (2):
>   Squashfs: Let the number of fragments cached configurable
>   Documentation/kernel-parameters.txt: Add kernel parameter of squashfs
>     fragments' cache size
>
>  Documentation/admin-guide/kernel-parameters.txt |  7 ++++
>  fs/squashfs/super.c                             | 43 ++++++++++++++++++++++++-
>  2 files changed, 49 insertions(+), 1 deletion(-)
>
> --
> 2.7.4
>