* [PATCH 0/2] Make squashfs fragments' cache size more configurable @ 2017-10-18 23:50 Qixuan Wu 2017-10-18 23:50 ` [PATCH 1/2] Squashfs: Let the number of fragments cached configurable Qixuan Wu ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Qixuan Wu @ 2017-10-18 23:50 UTC (permalink / raw) To: phillip, corbet Cc: paulmck, akpm, tglx, cdall, mingo, marc.zyngier, mchehab, zohar, linux-doc, linux-kernel, squashfs-devel, wuqixuan 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. 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] Squashfs: Let the number of fragments cached configurable 2017-10-18 23:50 [PATCH 0/2] Make squashfs fragments' cache size more configurable Qixuan Wu @ 2017-10-18 23:50 ` Qixuan Wu 2017-10-18 23:51 ` [PATCH 2/2] Documentation/kernel-parameters.txt: Add kernel parameter of squashfs fragments' cache size Qixuan Wu 2017-10-20 6:18 ` [PATCH 0/2] Make squashfs fragments' cache size more configurable Phillip Lougher 2 siblings, 0 replies; 5+ messages in thread From: Qixuan Wu @ 2017-10-18 23:50 UTC (permalink / raw) To: phillip, corbet Cc: paulmck, akpm, tglx, cdall, mingo, marc.zyngier, mchehab, zohar, linux-doc, linux-kernel, squashfs-devel, wuqixuan, Chen Jie 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. Now make it be configured during booting or inserting module. Signed-off-by: Qixuan Wu <wuqixuan@huawei.com> Signed-off-by: Chen Jie <chenjie6@huawei.com> --- fs/squashfs/super.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c index cf01e15..82f9e46 100644 --- a/fs/squashfs/super.c +++ b/fs/squashfs/super.c @@ -38,6 +38,7 @@ #include <linux/module.h> #include <linux/magic.h> #include <linux/xattr.h> +#include <linux/ctype.h> #include "squashfs_fs.h" #include "squashfs_fs_sb.h" @@ -49,6 +50,37 @@ static struct file_system_type squashfs_fs_type; static const struct super_operations squashfs_super_ops; +static unsigned int squashfs_frags_cache_size = SQUASHFS_CACHED_FRAGMENTS; + +#ifndef MODULE +static int __init squashfs_frags_cachesize_setup(char *buf) +{ + unsigned int arg; + + if (!buf || kstrtouint(buf, 10, &arg) < 0) { + pr_err("Bad squashfs fragments' cache size, let it be %d\n", + SQUASHFS_CACHED_FRAGMENTS); + return 0; + } + + if (arg < 3 || arg > 99) { + pr_warn("Squashfs fragments' cache size should be in the range " + "of [3,99]. Let it be %d\n", SQUASHFS_CACHED_FRAGMENTS); + return 0; + } + + squashfs_frags_cache_size = arg; + + return 0; +} +early_param("squashfs.frags_cache_size", squashfs_frags_cachesize_setup); + +#else +module_param(squashfs_frags_cache_size, uint, 0444); +MODULE_PARM_DESC(squashfs_frags_cache_size, + "Squashfs fragments' cache size, it should be in the range of [3,99]"); +#endif + static const struct squashfs_decompressor *supported_squashfs_filesystem(short major, short minor, short id) { @@ -276,8 +308,17 @@ static int squashfs_fill_super(struct super_block *sb, void *data, int silent) if (fragments == 0) goto check_directory_table; + if ((squashfs_frags_cache_size < 3) + || (squashfs_frags_cache_size > 99)) { + ERROR("Fragments' cache size should be in the range of[3,99]." + "Let it be %d\n", SQUASHFS_CACHED_FRAGMENTS); + squashfs_frags_cache_size = SQUASHFS_CACHED_FRAGMENTS; + } + + TRACE("Fragments' cache size %d\n", squashfs_frags_cache_size); + msblk->fragment_cache = squashfs_cache_init("fragment", - SQUASHFS_CACHED_FRAGMENTS, msblk->block_size); + squashfs_frags_cache_size, msblk->block_size); if (msblk->fragment_cache == NULL) { err = -ENOMEM; goto failed_mount; -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] Documentation/kernel-parameters.txt: Add kernel parameter of squashfs fragments' cache size 2017-10-18 23:50 [PATCH 0/2] Make squashfs fragments' cache size more configurable Qixuan Wu 2017-10-18 23:50 ` [PATCH 1/2] Squashfs: Let the number of fragments cached configurable Qixuan Wu @ 2017-10-18 23:51 ` Qixuan Wu 2017-10-20 6:18 ` [PATCH 0/2] Make squashfs fragments' cache size more configurable Phillip Lougher 2 siblings, 0 replies; 5+ messages in thread From: Qixuan Wu @ 2017-10-18 23:51 UTC (permalink / raw) To: phillip, corbet Cc: paulmck, akpm, tglx, cdall, mingo, marc.zyngier, mchehab, zohar, linux-doc, linux-kernel, squashfs-devel, wuqixuan 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. Now make it be configured during booting. Signed-off-by: Qixuan Wu <wuqixuan@huawei.com> --- Documentation/admin-guide/kernel-parameters.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 0549662..67bab3b 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3898,6 +3898,13 @@ spia_pedr= spia_peddr= + squashfs.frags_cache_size= + [SQUASHFS] The fragments' cache size. The value should + be in the range of [3,99]. + + The default value of this parameter is determined by + the config option CONFIG_SQUASHFS_FRAGMENT_CACHE_SIZE. + srcutree.counter_wrap_check [KNL] Specifies how frequently to check for grace-period sequence counter wrap for the -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] Make squashfs fragments' cache size more configurable 2017-10-18 23:50 [PATCH 0/2] Make squashfs fragments' cache size more configurable Qixuan Wu 2017-10-18 23:50 ` [PATCH 1/2] Squashfs: Let the number of fragments cached configurable Qixuan Wu 2017-10-18 23:51 ` [PATCH 2/2] Documentation/kernel-parameters.txt: Add kernel parameter of squashfs fragments' cache size Qixuan Wu @ 2017-10-20 6:18 ` Phillip Lougher 2017-10-20 10:45 ` Wuqixuan 2 siblings, 1 reply; 5+ messages in thread From: Phillip Lougher @ 2017-10-20 6:18 UTC (permalink / raw) To: Qixuan Wu Cc: Phillip Lougher, corbet, paulmck, Andrew Morton, tglx, cdall, mingo, marc.zyngier, mchehab, zohar, linux-doc, LKML, SquashFS developers On Thu, Oct 19, 2017 at 12:50 AM, Qixuan Wu <wuqixuan@huawei.com> 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] Make squashfs fragments' cache size more configurable 2017-10-20 6:18 ` [PATCH 0/2] Make squashfs fragments' cache size more configurable Phillip Lougher @ 2017-10-20 10:45 ` Wuqixuan 0 siblings, 0 replies; 5+ messages in thread From: Wuqixuan @ 2017-10-20 10:45 UTC (permalink / raw) To: Phillip Lougher Cc: Phillip Lougher, corbet, paulmck, Andrew Morton, tglx, cdall, mingo, marc.zyngier, mchehab, zohar, linux-doc, LKML, SquashFS developers Hi Phillip, Thank you for your fast reply. On Fri, Oct 20, 2017 at 2:18 PM, Phillip Lougher <phillip.lougher@gmail.com> wrote: > On Thu, Oct 19, 2017 at 12:50 AM, Qixuan Wu <wuqixuan@huawei.com> 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) ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-20 10:46 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-18 23:50 [PATCH 0/2] Make squashfs fragments' cache size more configurable Qixuan Wu 2017-10-18 23:50 ` [PATCH 1/2] Squashfs: Let the number of fragments cached configurable Qixuan Wu 2017-10-18 23:51 ` [PATCH 2/2] Documentation/kernel-parameters.txt: Add kernel parameter of squashfs fragments' cache size Qixuan Wu 2017-10-20 6:18 ` [PATCH 0/2] Make squashfs fragments' cache size more configurable Phillip Lougher 2017-10-20 10:45 ` Wuqixuan
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.