Thread (35 messages) 35 messages, 5 authors, 2017-05-17

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help