Received-SPF: pass (google.com: domain of linux-kernel+bounces-145323-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <e3115135-bc20-4bca-4ca1-72d8775ee706@huawei.com>
Date: Mon, 15 Apr 2024 22:08:28 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.1.2
Subject: Re: [PATCH] erofs: set SB_NODEV sb_flags when mounting with fsid
Content-Language: en-US
To: Christian Brauner <brauner@kernel.org>
CC: <linux-erofs@lists.ozlabs.org>, <xiang@kernel.org>, <chao@kernel.org>,
	<huyue2@coolpad.com>, <jefflexu@linux.alibaba.com>,
	<viro@zeniv.linux.org.uk>, <linux-kernel@vger.kernel.org>,
	<yangerkun@huawei.com>, <houtao1@huawei.com>, Baokun Li
	<libaokun1@huawei.com>
References: <20240415121746.1207242-1-libaokun1@huawei.com>
 <20240415-betagten-querlatte-feb727ed56c1@brauner>
From: Baokun Li <libaokun1@huawei.com>
In-Reply-To: <20240415-betagten-querlatte-feb727ed56c1@brauner>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

On 2024/4/15 21:38, Christian Brauner wrote:
> On Mon, Apr 15, 2024 at 08:17:46PM +0800, Baokun Li wrote:
>> When erofs_kill_sb() is called in block dev based mode, s_bdev may not have
>> been initialised yet, and if CONFIG_EROFS_FS_ONDEMAND is enabled, it will
>> be mistaken for fscache mode, and then attempt to free an anon_dev that has
>> never been allocated, triggering the following warning:
>>
>> ============================================
>> ida_free called for id=0 which is not allocated.
>> WARNING: CPU: 14 PID: 926 at lib/idr.c:525 ida_free+0x134/0x140
>> Modules linked in:
>> CPU: 14 PID: 926 Comm: mount Not tainted 6.9.0-rc3-dirty #630
>> RIP: 0010:ida_free+0x134/0x140
>> Call Trace:
>>   <TASK>
>>   erofs_kill_sb+0x81/0x90
>>   deactivate_locked_super+0x35/0x80
>>   get_tree_bdev+0x136/0x1e0
>>   vfs_get_tree+0x2c/0xf0
>>   do_new_mount+0x190/0x2f0
>>   [...]
>> ============================================
>>
>> To avoid this problem, add SB_NODEV to fc->sb_flags after successfully
>> parsing the fsid, and then the superblock inherits this flag when it is
>> allocated, so that the sb_flags can be used to distinguish whether it is
>> in block dev based mode when calling erofs_kill_sb().
>>
>> Signed-off-by: Baokun Li <libaokun1@huawei.com>
>> ---
>>   fs/erofs/super.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index b21bd8f78dc1..7539ce7d64bc 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -520,6 +520,7 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>>   		ctx->fsid = kstrdup(param->string, GFP_KERNEL);
>>   		if (!ctx->fsid)
>>   			return -ENOMEM;
>> +		fc->sb_flags |= SB_NODEV;
> Hm, I wouldn't do it this way. That's an abuse of that flag imho.
> Record the information in the erofs_fs_context if you need to.
Hi Christian!

The problem here is that when mounting erofs, if we have an fsid
then it is not block device based, if we don't have an fsid it is block
device based. So only after we confirmed whether we have an fsid
or not, we can confirm whether we need SB_NODEV or not.

-- 
With Best Regards,
Baokun Li
.