Re: [PATCH 16/25] fuse: Convert to separately allocated bdi
From: Rakesh Pandit <hidden>
Date: 2017-05-16 18:37:36
Also in:
linux-fsdevel
On Tue, May 16, 2017 at 12:48:31PM +0200, Jan Kara wrote:
On Mon 15-05-17 23:34:00, Rakesh Pandit wrote:quoted
Hi Jan, Miklos, On Wed, Apr 12, 2017 at 12:24:40PM +0200, Jan Kara wrote:quoted
Allocate struct backing_dev_info separately instead of embedding it inside the superblock. This unifies handling of bdi among users.
....
quoted
...quoted
static int fuse_bdi_init(struct fuse_conn *fc, struct super_block *sb) { int err; + char *suffix = ""; - fc->bdi.name = "fuse"; - fc->bdi.ra_pages = (VM_MAX_READAHEAD * 1024) / PAGE_SIZE; - /* fuse does it's own writeback accounting */ - fc->bdi.capabilities = BDI_CAP_NO_ACCT_WB | BDI_CAP_STRICTLIMIT; - - err = bdi_init(&fc->bdi); + if (sb->s_bdev) + suffix = "-fuseblk"; + err = super_setup_bdi_name(sb, "%u:%u%s", MAJOR(fc->dev), + MINOR(fc->dev), suffix); if (err) return err;This call to super_setup_bdi_name would only work with "fuse" but not with "fuseblk" as mounting a block device in userspace triggers mount_bdev call which results in set_bdev_super taking a reference from block device's BDI. But super_setup_bdi_name allocates a new bdi and ignores the already existing reference which triggers: WARN_ON(sb->s_bdi != &noop_backing_dev_info); as sb->s_bdi already has a reference from set_bdev_super. This works for "fuse" (without a blocking device) for obvious reasons. I can reproduce this on -rc1 and also found a report on lkml: https://lkml.org/lkml/2017/5/2/445 Only sane solution seems to be maintaining a private bdi instace just for fuseblk and let fuse use the common new infrastructure.Thanks for analysis! Does the attached patch fix the warning for you?
Yes, tested. Feel free to add: Tested-by: Rakesh Pandit <redacted>
quoted hunk ↗ jump to hunk
From 5b0cfc37b45670a35228c96cbaee2b99cd3d447c Mon Sep 17 00:00:00 2001 From: Jan Kara <jack@suse.cz> Date: Tue, 16 May 2017 12:22:22 +0200 Subject: [PATCH] fuseblk: Fix warning in super_setup_bdi_name() Commit 5f7f7543f52e "fuse: Convert to separately allocated bdi" didn't properly handle fuseblk filesystem. When fuse_bdi_init() is called for that filesystem type, sb->s_bdi is already initialized (by set_bdev_super()) to point to block device's bdi and consequently super_setup_bdi_name() complains about this fact when reseting bdi to the private one. Fix the problem by properly dropping bdi reference in fuse_bdi_init() before creating a private bdi in super_setup_bdi_name(). Fixes: 5f7f7543f52eee03ed35c9d671fbb1cdbd4bc9b5 Reported-by: Rakesh Pandit <redacted> Signed-off-by: Jan Kara <jack@suse.cz> --- fs/fuse/inode.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index 5a1b58f8fef4..65c88379a3a1 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c@@ -975,8 +975,15 @@ static int fuse_bdi_init(struct fuse_conn *fc, struct super_block *sb) int err; char *suffix = ""; - if (sb->s_bdev) + if (sb->s_bdev) { suffix = "-fuseblk"; + /* + * sb->s_bdi points to blkdev's bdi however we want to redirect + * it to our private bdi... + */ + bdi_put(sb->s_bdi); + sb->s_bdi = &noop_backing_dev_info; + } err = super_setup_bdi_name(sb, "%u:%u%s", MAJOR(fc->dev), MINOR(fc->dev), suffix); if (err)-- 2.12.0