On 23/10/30 03:40PM, Phillip Lougher wrote: > On 30/10/2023 12:57, Ariel Miculas wrote: > > On 23/10/29 08:33PM, Phillip Lougher wrote: > > > On 29/10/2023 16:19, Ariel Miculas wrote: > > > > When SQUASHFS_CHOICE_DECOMP_BY_MOUNT is set, the "threads" mount option > > > > can be used to specify the decompression mode: single-threaded, > > > > multi-threaded, percpu or the number of threads used for decompression. > > > > When SQUASHFS_CHOICE_DECOMP_BY_MOUNT is not set and > > > > SQUASHFS_DECOMP_MULTI is set, the "threads" option can also be used to > > > > specify the number of threads used for decompression. This mount option > > > > is only mentioned in fs/squashfs/Kconfig, which makes it difficult to > > > > find. > > > > > > > > Another mount option available is "errors", which can be configured to > > > > panic the kernel when squashfs errors are encountered. > > > > > > > > Add both these options to the squashfs documentation, making them more > > > > noticeable. > > > > > > > > Signed-off-by: Ariel Miculas <amiculas@xxxxxxxxx> > > > > > > Looks good to me. > > > > > > Reviewed-by: Phillip Lougher <phillip@xxxxxxxxxxxxxxx> > > > > Unfortunately, it seems this is not quite correct either: > > There is the config option SQUASHFS_MOUNT_DECOMP_THREADS: > > ``` > > bool "Add the mount parameter 'threads=' for squashfs" > > depends on SQUASHFS > > depends on SQUASHFS_DECOMP_MULTI > > default n > > help > > Use threads= to set the decompression parallel mode and the number of threads. > > If SQUASHFS_CHOICE_DECOMP_BY_MOUNT=y > > threads=<single|multi|percpu|1|2|3|...> > > else > > threads=<2|3|...> > > The upper limit is num_online_cpus() * 2. > > ``` > > that depends on SQUASHFS_DECOMP_MULTI. > > So I think I should take my v1 patch and specify that the "threads=" > > mount option depends on SQUASHFS_MOUNT_DECOMP_THREADS. There's no need > > to specify SQUASHFS_DECOMP_MULTI, because SQUASHFS_MOUNT_DECOMP_THREADS > > already depends on it. > > Sorry, you have to specify SQUASHFS_DECOMP_MULTI to be able to specify > SQUASHFS_MOUNT_DECOMP_THREADS if SQUASHFS_DECOMP_BY_MOUNT is unselected. Agree. > > Just try it, do make menuconfig, ensure SQUASHFS_CHOICE_DECOMP_BY_MOUNT > is unselected, select Single threaded decompression and you won't be > able to specify SQUASHFS_MOUNT_DECOMP_THREADS. True. > > That was the point of my review. What bit don't you understand? But SQUASHFS_DECOMP_MULTI is not enough, you need to specify SQUASHFS_MOUNT_DECOMP_THREADS in order to use the "threads=" mount option. So instead of saying ``` If SQUASHFS_CHOICE_DECOMP_BY_MOUNT is **not** set and SQUASHFS_DECOMP_MULTI is set: ``` wouldn't it be right to actually say: ``` If SQUASHFS_CHOICE_DECOMP_BY_MOUNT is **not** set and SQUASHFS_MOUNT_DECOMP_THREADS is set: ```? As you've mentioned, you could only set SQUASHFS_MOUNT_DECOMP_THREADS when SQUASHFS_DECOMP_MULTI is selected. That happens in two cases: * either SQUASHFS_CHOICE_DECOMP_BY_MOUNT is set, in which case it also selects SQUASHFS_MOUNT_DECOMP_THREADS * either SQUASHFS_CHOICE_DECOMP_BY_MOUNT is not set, SQUASHFS_DECOMP_MULTI is set and SQUASHFS_MOUNT_DECOMP_THREADS is also set So I wouldn't even mention SQUASHFS_DECOMP_MULTI in the documentation, only SQUASHFS_MOUNT_DECOMP_THREADS, because the latter always depends on the former. And the "threads=" mount option is only available when SQUASHFS_MOUNT_DECOMP_THREADS is set (which is the configuration I've missed in v1 and v2). Regards, Ariel > > Phillip > > > What do you think? > > > > Regards, > > Ariel > > >
