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

[Wuqixuan]

Hi Phillip, 

Thank you for your fast reply. 

On Fri, Oct 20, 2017 at 2:18 PM,  Phillip Lougher ‎<phillip.lougher@xxxxxxxxx> wrote:
> 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.

I need apologize for not describing the scenario clear enough. In our 
company, maybe we have a bit different kernel distribution mode. We 
only can release the single kernel binary to multiple customer. For one
customer of us, they have a very strict kernel boot speed requirement that
is 2~3s including rootfs (squashfs) uncompression. We found if modify the 
value from 3 to 8, some handreds of miliseconds can be saved, and the total 
boot time satisfied the requirement. But we were afraid to impact other 
customer, so used kernel boot parameter. Module interface currently there 
is no user-case. 

Maybe it's not correct, from my opinion, that kernel boot parameter is almost 
same as config option, kernel module, /proc or /sysfs. It gives administrator 
the chance to change some kernel's variables as per different scenario, if 
they cannot chagne the config option. And administrator sometime is root, 
not normal user. So the parameter set by them through kernel boot parameter 
with enough understanding, testing and analysis. For example, such kernel boot
parameters like crashkernel=size[KMG], default_hugepagesz= also do the 
same work. So before setting to 8, the administrator of our customer understands 
the meaning and memory overhead impact of this modification of fragments 
cache size. 

Frankly I admit maybe our scenario is a bit special in embedded system, it's 
not as useful for others as for us. So it seems like a bit over-design and I can 
understand what you are worried about if accepting the feature. 

Anyway, thanks for your reply and your opinion. 

Thanks 
Qixuan

> Phillip Lougher (Squashfs maintainer)