Hi Christian, Baokun, On Mon, Apr 15, 2024 at 11:23:58PM +0800, Baokun Li wrote: > 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 <libaok...@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. > The stack diagram that triggers the problem is as follows, the call to > erofs_kill_sb() fails before fill_super() has been executed, and we can > only use super_block to determine whether it is currently in nodev > fscahe mode or block device based mode. So if it is recorded in > erofs_fs_context (aka fc->fs_private), we can't access the recorded data > unless we pass fc into erofs_kill_sb() as well. >
If I understand correctly, from the discussion above, I think there exists a gap between alloc_super() and sb->s_bdev is set. But .kill_sb() can be called between them and fc is not passed into .kill_sb(). I'm not sure how to resolve it in EROFS itself, anyway... Thanks, Gao Xiang
