Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Message-ID: <46d308b2-5e59-862d-b429-e6ee418bc61e@squashfs.org.uk>
Date:   Mon, 30 Oct 2023 15:40:55 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH v2] docs: filesystems: document the squashfs specific
 mount options
To:     Ariel Miculas <amiculas@cisco.com>
Cc:     linux-doc@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>, serge@hallyn.com,
        Jonathan Corbet <corbet@lwn.net>, linux-kernel@vger.kernel.org
References: <20231029161924.50648-1-amiculas@cisco.com>
 <bd039dc7-cfe5-c210-b65e-0cba3186ee0b@squashfs.org.uk>
 <qgwx4cjswrhzxqijsosix437xyw3ifglv6uz2d3nq7whkjqueo@c5pzoqtoqtgb>
From:   Phillip Lougher <phillip@squashfs.org.uk>
In-Reply-To: <qgwx4cjswrhzxqijsosix437xyw3ifglv6uz2d3nq7whkjqueo@c5pzoqtoqtgb>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@cisco.com>
>>
>> Looks good to me.
>>
>> Reviewed-by: Phillip Lougher <phillip@squashfs.org.uk>
> 
> 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.

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.

That was the point of my review. What bit don't you understand?

Phillip

> What do you think?
> 
> Regards,
> Ariel
>