[PATCH 0/24 RFC] fs: Convert all embedded bdis into separate ones

[PATCH 0/24 RFC] fs: Convert all embedded bdis into separate ones

[Jan Kara]

Hello,

this patch series converts all embedded occurences of struct backing_dev_info
to use standalone dynamically allocated structures. This makes bdi handling
unified across all bdi users and generally removes some boilerplate code from
filesystems setting up their own bdi. It also allows us to remove some code
from generic bdi implementation.

The patches were only compile-tested for most filesystems (I've tested
mounting only for NFS & btrfs) so fs maintainers please have a look whether
the changes look sound to you.

This series is based on top of bdi fixes that were merged into linux-block
git tree.

								Honza

[PATCH 04/24] fs: Provide infrastructure for dynamic BDIs in filesystems

[Jan Kara]

Provide helper functions for setting up dynamically allocated
backing_dev_info structures for filesystems and cleaning them up on
superblock destruction.

CC: linux-mtd@lists.infradead.org
CC: linux-nfs@vger.kernel.org
CC: Petr Vandrovec <petr@vandrovec.name>
CC: linux-nilfs@vger.kernel.org
CC: cluster-devel@redhat.com
CC: osd-dev@open-osd.org
CC: codalist@coda.cs.cmu.edu
CC: linux-afs@lists.infradead.org
CC: ecryptfs@vger.kernel.org
CC: linux-cifs@vger.kernel.org
CC: ceph-devel@vger.kernel.org
CC: linux-btrfs@vger.kernel.org
CC: v9fs-developer@lists.sourceforge.net
CC: lustre-devel@lists.lustre.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/super.c                       | 49 ++++++++++++++++++++++++++++++++++++++++
 include/linux/backing-dev-defs.h |  2 +-
 include/linux/fs.h               |  6 +++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/fs/super.c b/fs/super.c
index ea662b0e5e78..31dc4c6450ef 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -446,6 +446,11 @@ void generic_shutdown_super(struct super_block *sb)
 	hlist_del_init(&sb->s_instances);
 	spin_unlock(&sb_lock);
 	up_write(&sb->s_umount);
+	if (sb->s_iflags & SB_I_DYNBDI) {
+		bdi_put(sb->s_bdi);
+		sb->s_bdi = &noop_backing_dev_info;
+		sb->s_iflags &= ~SB_I_DYNBDI;
+	}
 }
 
 EXPORT_SYMBOL(generic_shutdown_super);
@@ -1249,6 +1254,50 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data)
 }
 
 /*
+ * Setup private BDI for given superblock. I gets automatically cleaned up
+ * in generic_shutdown_super().
+ */
+int super_setup_bdi_name(struct super_block *sb, char *fmt, ...)
+{
+	struct backing_dev_info *bdi;
+	int err;
+	va_list args;
+
+	bdi = bdi_alloc(GFP_KERNEL);
+	if (!bdi)
+		return -ENOMEM;
+
+	bdi->name = sb->s_type->name;
+
+	va_start(args, fmt);
+	err = bdi_register_va(bdi, NULL, fmt, args);
+	va_end(args);
+	if (err) {
+		bdi_put(bdi);
+		return err;
+	}
+	WARN_ON(sb->s_bdi != &noop_backing_dev_info);
+	sb->s_bdi = bdi;
+	sb->s_iflags |= SB_I_DYNBDI;
+
+	return 0;
+}
+EXPORT_SYMBOL(super_setup_bdi_name);
+
+/*
+ * Setup private BDI for given superblock. I gets automatically cleaned up
+ * in generic_shutdown_super().
+ */
+int super_setup_bdi(struct super_block *sb)
+{
+	static atomic_long_t bdi_seq = ATOMIC_LONG_INIT(0);
+
+	return super_setup_bdi_name(sb, "%.28s-%ld", sb->s_type->name,
+				    atomic_long_inc_return(&bdi_seq));
+}
+EXPORT_SYMBOL(super_setup_bdi);
+
+/*
  * This is an internal function, please use sb_end_{write,pagefault,intwrite}
  * instead.
  */
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 2ecafc8a2d06..70080b4217f4 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -143,7 +143,7 @@ struct backing_dev_info {
 	congested_fn *congested_fn; /* Function pointer if device is md/dm */
 	void *congested_data;	/* Pointer to aux data for congested func */
 
-	char *name;
+	const char *name;
 
 	struct kref refcnt;	/* Reference counter for the structure */
 	unsigned int registered:1;	/* Is bdi registered? */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c930cbc19342..8ed8b6d1bc54 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1267,6 +1267,9 @@ struct mm_struct;
 /* sb->s_iflags to limit user namespace mounts */
 #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
 
+/* Temporary flag until all filesystems are converted to dynamic bdis */
+#define SB_I_DYNBDI	0x00000100
+
 /* Possible states of 'frozen' field */
 enum {
 	SB_UNFROZEN = 0,		/* FS is unfrozen */
@@ -2103,6 +2106,9 @@ extern int vfs_ustat(dev_t, struct kstatfs *);
 extern int freeze_super(struct super_block *super);
 extern int thaw_super(struct super_block *super);
 extern bool our_mnt(struct vfsmount *mnt);
+extern __printf(2, 3)
+int super_setup_bdi_name(struct super_block *sb, char *fmt, ...);
+extern int super_setup_bdi(struct super_block *sb);
 
 extern int current_umask(void);
 
-- 
2.10.2

[PATCH 19/24] nilfs2: Convert to properly refcounting bdi

[Jan Kara]

Similarly to set_bdev_super() NILFS2 just used block device reference to
bdi. Convert it to properly getting bdi reference. The reference will
get automatically dropped on superblock destruction.

CC: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
CC: linux-nilfs@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/nilfs2/super.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index e1872f36147f..feb796a38b8d 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -1068,7 +1068,8 @@ nilfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_time_gran = 1;
 	sb->s_max_links = NILFS_LINK_MAX;
 
-	sb->s_bdi = bdev_get_queue(sb->s_bdev)->backing_dev_info;
+	sb->s_bdi = bdi_get(sb->s_bdev->bd_bdi);
+	sb->s_iflags |= SB_I_DYNBDI;
 
 	err = load_nilfs(nilfs, sb);
 	if (err)
-- 
2.10.2

Re: [PATCH 04/24] fs: Provide infrastructure for dynamic BDIs in filesystems

[Liu Bo]

Hi,

On Thu, Feb 02, 2017 at 06:34:02PM +0100, Jan Kara wrote:
> Provide helper functions for setting up dynamically allocated
> backing_dev_info structures for filesystems and cleaning them up on
> superblock destruction.

Just one concern, will this cause problems for multiple superblock cases
like nfs with nosharecache?

Thanks,

-liubo

> 
> CC: linux-mtd@lists.infradead.org
> CC: linux-nfs@vger.kernel.org
> CC: Petr Vandrovec <petr@vandrovec.name>
> CC: linux-nilfs@vger.kernel.org
> CC: cluster-devel@redhat.com
> CC: osd-dev@open-osd.org
> CC: codalist@coda.cs.cmu.edu
> CC: linux-afs@lists.infradead.org
> CC: ecryptfs@vger.kernel.org
> CC: linux-cifs@vger.kernel.org
> CC: ceph-devel@vger.kernel.org
> CC: linux-btrfs@vger.kernel.org
> CC: v9fs-developer@lists.sourceforge.net
> CC: lustre-devel@lists.lustre.org
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/super.c                       | 49 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/backing-dev-defs.h |  2 +-
>  include/linux/fs.h               |  6 +++++
>  3 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index ea662b0e5e78..31dc4c6450ef 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -446,6 +446,11 @@ void generic_shutdown_super(struct super_block *sb)
>  	hlist_del_init(&sb->s_instances);
>  	spin_unlock(&sb_lock);
>  	up_write(&sb->s_umount);
> +	if (sb->s_iflags & SB_I_DYNBDI) {
> +		bdi_put(sb->s_bdi);
> +		sb->s_bdi = &noop_backing_dev_info;
> +		sb->s_iflags &= ~SB_I_DYNBDI;
> +	}
>  }
>  
>  EXPORT_SYMBOL(generic_shutdown_super);
> @@ -1249,6 +1254,50 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data)
>  }
>  
>  /*
> + * Setup private BDI for given superblock. I gets automatically cleaned up
> + * in generic_shutdown_super().
> + */
> +int super_setup_bdi_name(struct super_block *sb, char *fmt, ...)
> +{
> +	struct backing_dev_info *bdi;
> +	int err;
> +	va_list args;
> +
> +	bdi = bdi_alloc(GFP_KERNEL);
> +	if (!bdi)
> +		return -ENOMEM;
> +
> +	bdi->name = sb->s_type->name;
> +
> +	va_start(args, fmt);
> +	err = bdi_register_va(bdi, NULL, fmt, args);
> +	va_end(args);
> +	if (err) {
> +		bdi_put(bdi);
> +		return err;
> +	}
> +	WARN_ON(sb->s_bdi != &noop_backing_dev_info);
> +	sb->s_bdi = bdi;
> +	sb->s_iflags |= SB_I_DYNBDI;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(super_setup_bdi_name);
> +
> +/*
> + * Setup private BDI for given superblock. I gets automatically cleaned up
> + * in generic_shutdown_super().
> + */
> +int super_setup_bdi(struct super_block *sb)
> +{
> +	static atomic_long_t bdi_seq = ATOMIC_LONG_INIT(0);
> +
> +	return super_setup_bdi_name(sb, "%.28s-%ld", sb->s_type->name,
> +				    atomic_long_inc_return(&bdi_seq));
> +}
> +EXPORT_SYMBOL(super_setup_bdi);
> +
> +/*
>   * This is an internal function, please use sb_end_{write,pagefault,intwrite}
>   * instead.
>   */
> diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
> index 2ecafc8a2d06..70080b4217f4 100644
> --- a/include/linux/backing-dev-defs.h
> +++ b/include/linux/backing-dev-defs.h
> @@ -143,7 +143,7 @@ struct backing_dev_info {
>  	congested_fn *congested_fn; /* Function pointer if device is md/dm */
>  	void *congested_data;	/* Pointer to aux data for congested func */
>  
> -	char *name;
> +	const char *name;
>  
>  	struct kref refcnt;	/* Reference counter for the structure */
>  	unsigned int registered:1;	/* Is bdi registered? */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c930cbc19342..8ed8b6d1bc54 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1267,6 +1267,9 @@ struct mm_struct;
>  /* sb->s_iflags to limit user namespace mounts */
>  #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
>  
> +/* Temporary flag until all filesystems are converted to dynamic bdis */
> +#define SB_I_DYNBDI	0x00000100
> +
>  /* Possible states of 'frozen' field */
>  enum {
>  	SB_UNFROZEN = 0,		/* FS is unfrozen */
> @@ -2103,6 +2106,9 @@ extern int vfs_ustat(dev_t, struct kstatfs *);
>  extern int freeze_super(struct super_block *super);
>  extern int thaw_super(struct super_block *super);
>  extern bool our_mnt(struct vfsmount *mnt);
> +extern __printf(2, 3)
> +int super_setup_bdi_name(struct super_block *sb, char *fmt, ...);
> +extern int super_setup_bdi(struct super_block *sb);
>  
>  extern int current_umask(void);
>  
> -- 
> 2.10.2
> 

Re: [PATCH 04/24] fs: Provide infrastructure for dynamic BDIs in filesystems

[Jan Kara]

On Thu 02-02-17 11:28:27, Liu Bo wrote:
> Hi,
> 
> On Thu, Feb 02, 2017 at 06:34:02PM +0100, Jan Kara wrote:
> > Provide helper functions for setting up dynamically allocated
> > backing_dev_info structures for filesystems and cleaning them up on
> > superblock destruction.
> 
> Just one concern, will this cause problems for multiple superblock cases
> like nfs with nosharecache?

Can you ellaborate a bit? I've looked for a while what nfs with
nosharecache does but I didn't see how it would influence anything with
bdis...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 04/24] fs: Provide infrastructure for dynamic BDIs in filesystems

[Liu Bo]

On Fri, Feb 03, 2017 at 02:50:42PM +0100, Jan Kara wrote:
> On Thu 02-02-17 11:28:27, Liu Bo wrote:
> > Hi,
> > 
> > On Thu, Feb 02, 2017 at 06:34:02PM +0100, Jan Kara wrote:
> > > Provide helper functions for setting up dynamically allocated
> > > backing_dev_info structures for filesystems and cleaning them up on
> > > superblock destruction.
> > 
> > Just one concern, will this cause problems for multiple superblock cases
> > like nfs with nosharecache?
> 
> Can you ellaborate a bit? I've looked for a while what nfs with
> nosharecache does but I didn't see how it would influence anything with
> bdis...

Oh, I missed that bdi_seq was static, then it should be fine.

(I was worried about that nfs with nosharecache would have multiple
superblocks and if each superblock has a bdi using the same bdi name,
nfs-xx.)

Thanks for the reply.

Thanks,

-liubo

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 04/24] fs: Provide infrastructure for dynamic BDIs in filesystems

[Dilger, Andreas]

On Feb 2, 2017, at 10:34, Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org> wrote:
> 
> Provide helper functions for setting up dynamically allocated
> backing_dev_info structures for filesystems and cleaning them up on
> superblock destruction.
> 
> CC: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> CC: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: Petr Vandrovec <petr-vPk2MGR0e28uaRcfnNAh7A@public.gmane.org>
> CC: linux-nilfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: cluster-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> CC: osd-dev-yNzVSZO3znNg9hUCZPvPmw@public.gmane.org
> CC: codalist-/uMB558Y47wP4a1z8dhFYw@public.gmane.org
> CC: linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> CC: ecryptfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: linux-btrfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> CC: lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org
> Signed-off-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
> ---
> fs/super.c                       | 49 ++++++++++++++++++++++++++++++++++++++++
> include/linux/backing-dev-defs.h |  2 +-
> include/linux/fs.h               |  6 +++++
> 3 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index ea662b0e5e78..31dc4c6450ef 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -446,6 +446,11 @@ void generic_shutdown_super(struct super_block *sb)
> 	hlist_del_init(&sb->s_instances);
> 	spin_unlock(&sb_lock);
> 	up_write(&sb->s_umount);
> +	if (sb->s_iflags & SB_I_DYNBDI) {
> +		bdi_put(sb->s_bdi);
> +		sb->s_bdi = &noop_backing_dev_info;
> +		sb->s_iflags &= ~SB_I_DYNBDI;
> +	}
> }
> 
> EXPORT_SYMBOL(generic_shutdown_super);
> @@ -1249,6 +1254,50 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data)
> }
> 
> /*
> + * Setup private BDI for given superblock. I gets automatically cleaned up

(typo) s/I/It/

Looks fine otherwise.

> + * in generic_shutdown_super().
> + */
> +int super_setup_bdi_name(struct super_block *sb, char *fmt, ...)
> +{
> +	struct backing_dev_info *bdi;
> +	int err;
> +	va_list args;
> +
> +	bdi = bdi_alloc(GFP_KERNEL);
> +	if (!bdi)
> +		return -ENOMEM;
> +
> +	bdi->name = sb->s_type->name;
> +
> +	va_start(args, fmt);
> +	err = bdi_register_va(bdi, NULL, fmt, args);
> +	va_end(args);
> +	if (err) {
> +		bdi_put(bdi);
> +		return err;
> +	}
> +	WARN_ON(sb->s_bdi != &noop_backing_dev_info);
> +	sb->s_bdi = bdi;
> +	sb->s_iflags |= SB_I_DYNBDI;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(super_setup_bdi_name);
> +
> +/*
> + * Setup private BDI for given superblock. I gets automatically cleaned up
> + * in generic_shutdown_super().
> + */
> +int super_setup_bdi(struct super_block *sb)
> +{
> +	static atomic_long_t bdi_seq = ATOMIC_LONG_INIT(0);
> +
> +	return super_setup_bdi_name(sb, "%.28s-%ld", sb->s_type->name,
> +				    atomic_long_inc_return(&bdi_seq));
> +}
> +EXPORT_SYMBOL(super_setup_bdi);
> +
> +/*
>  * This is an internal function, please use sb_end_{write,pagefault,intwrite}
>  * instead.
>  */
> diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
> index 2ecafc8a2d06..70080b4217f4 100644
> --- a/include/linux/backing-dev-defs.h
> +++ b/include/linux/backing-dev-defs.h
> @@ -143,7 +143,7 @@ struct backing_dev_info {
> 	congested_fn *congested_fn; /* Function pointer if device is md/dm */
> 	void *congested_data;	/* Pointer to aux data for congested func */
> 
> -	char *name;
> +	const char *name;
> 
> 	struct kref refcnt;	/* Reference counter for the structure */
> 	unsigned int registered:1;	/* Is bdi registered? */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c930cbc19342..8ed8b6d1bc54 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1267,6 +1267,9 @@ struct mm_struct;
> /* sb->s_iflags to limit user namespace mounts */
> #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
> 
> +/* Temporary flag until all filesystems are converted to dynamic bdis */
> +#define SB_I_DYNBDI	0x00000100
> +
> /* Possible states of 'frozen' field */
> enum {
> 	SB_UNFROZEN = 0,		/* FS is unfrozen */
> @@ -2103,6 +2106,9 @@ extern int vfs_ustat(dev_t, struct kstatfs *);
> extern int freeze_super(struct super_block *super);
> extern int thaw_super(struct super_block *super);
> extern bool our_mnt(struct vfsmount *mnt);
> +extern __printf(2, 3)
> +int super_setup_bdi_name(struct super_block *sb, char *fmt, ...);
> +extern int super_setup_bdi(struct super_block *sb);
> 
> extern int current_umask(void);
> 
> -- 
> 2.10.2
> 
> _______________________________________________
> lustre-devel mailing list
> lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation

Re: [PATCH 04/24] fs: Provide infrastructure for dynamic BDIs in filesystems

[Jan Kara]

> > @@ -1249,6 +1254,50 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data)
> > }
> > 
> > /*
> > + * Setup private BDI for given superblock. I gets automatically cleaned up
> 
> (typo) s/I/It/
> 
> Looks fine otherwise.

Thanks, fixed.

								Honza
-- 
Jan Kara <jack-IBi9RG/b67k@public.gmane.org>
SUSE Labs, CR