[22/24] ubifs: Convert to separately allocated bdi

Submitted by Jan Kara on Feb. 2, 2017, 5:34 p.m.

Details

Message ID 20170202173422.3240-23-jack@suse.cz
State New
Headers show

Commit Message

Jan Kara Feb. 2, 2017, 5:34 p.m.
Allocate struct backing_dev_info separately instead of embedding it
inside the superblock. This unifies handling of bdi among users.

CC: Richard Weinberger <richard@nod.at>
CC: Artem Bityutskiy <dedekind1@gmail.com>
CC: Adrian Hunter <adrian.hunter@intel.com>
CC: linux-mtd@lists.infradead.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ubifs/super.c | 23 +++++++----------------
 fs/ubifs/ubifs.h |  3 ---
 2 files changed, 7 insertions(+), 19 deletions(-)

Comments

Richard Weinberger Feb. 2, 2017, 8:34 p.m.
Jan,

Am 02.02.2017 um 18:34 schrieb Jan Kara:
> Allocate struct backing_dev_info separately instead of embedding it
> inside the superblock. This unifies handling of bdi among users.
> 
> CC: Richard Weinberger <richard@nod.at>
> CC: Artem Bityutskiy <dedekind1@gmail.com>
> CC: Adrian Hunter <adrian.hunter@intel.com>
> CC: linux-mtd@lists.infradead.org
> Signed-off-by: Jan Kara <jack@suse.cz>

Is this series available at some git tree, please?

Thanks,
//richard
Jan Kara Feb. 3, 2017, 1:45 p.m.
On Thu 02-02-17 21:34:32, Richard Weinberger wrote:
> Jan,
> 
> Am 02.02.2017 um 18:34 schrieb Jan Kara:
> > Allocate struct backing_dev_info separately instead of embedding it
> > inside the superblock. This unifies handling of bdi among users.
> > 
> > CC: Richard Weinberger <richard@nod.at>
> > CC: Artem Bityutskiy <dedekind1@gmail.com>
> > CC: Adrian Hunter <adrian.hunter@intel.com>
> > CC: linux-mtd@lists.infradead.org
> > Signed-off-by: Jan Kara <jack@suse.cz>
> 
> Is this series available at some git tree, please?

I've pushed it out to:

git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git bdi

								Honza
Richard Weinberger Feb. 8, 2017, 11:24 a.m.
Am 02.02.2017 um 18:34 schrieb Jan Kara:
> Allocate struct backing_dev_info separately instead of embedding it
> inside the superblock. This unifies handling of bdi among users.
> 
> CC: Richard Weinberger <richard@nod.at>
> CC: Artem Bityutskiy <dedekind1@gmail.com>
> CC: Adrian Hunter <adrian.hunter@intel.com>
> CC: linux-mtd@lists.infradead.org
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ubifs/super.c | 23 +++++++----------------
>  fs/ubifs/ubifs.h |  3 ---
>  2 files changed, 7 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index e08aa04fc835..34810eb52b22 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1827,7 +1827,6 @@ static void ubifs_put_super(struct super_block *sb)
>  	}
>  
>  	ubifs_umount(c);
> -	bdi_destroy(&c->bdi);
>  	ubi_close_volume(c->ubi);
>  	mutex_unlock(&c->umount_mutex);
>  }
> @@ -2019,29 +2018,23 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
>  		goto out;
>  	}
>  
> +	err = ubifs_parse_options(c, data, 0);
> +	if (err)
> +		goto out_close;
> +
>  	/*
>  	 * UBIFS provides 'backing_dev_info' in order to disable read-ahead. For
>  	 * UBIFS, I/O is not deferred, it is done immediately in readpage,
>  	 * which means the user would have to wait not just for their own I/O
>  	 * but the read-ahead I/O as well i.e. completely pointless.
>  	 *
> -	 * Read-ahead will be disabled because @c->bdi.ra_pages is 0.
> +	 * Read-ahead will be disabled because @sb->s_bdi->ra_pages is 0.
>  	 */
> -	c->bdi.name = "ubifs",
> -	c->bdi.capabilities = 0;

So ->capabilities is now zero by default since you use __GFP_ZERO in
bdi_alloc().
At least for UBIFS I'll add a comment on this, otherwise it is not so
clear that UBIFS wants a BDI with no capabilities and how it achieves that.

Acked-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard
Jan Kara Feb. 9, 2017, 12:17 p.m.
On Wed 08-02-17 12:24:00, Richard Weinberger wrote:
> Am 02.02.2017 um 18:34 schrieb Jan Kara:
> > Allocate struct backing_dev_info separately instead of embedding it
> > inside the superblock. This unifies handling of bdi among users.
> > 
> > CC: Richard Weinberger <richard@nod.at>
> > CC: Artem Bityutskiy <dedekind1@gmail.com>
> > CC: Adrian Hunter <adrian.hunter@intel.com>
> > CC: linux-mtd@lists.infradead.org
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/ubifs/super.c | 23 +++++++----------------
> >  fs/ubifs/ubifs.h |  3 ---
> >  2 files changed, 7 insertions(+), 19 deletions(-)
> > 
> > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> > index e08aa04fc835..34810eb52b22 100644
> > --- a/fs/ubifs/super.c
> > +++ b/fs/ubifs/super.c
> > @@ -1827,7 +1827,6 @@ static void ubifs_put_super(struct super_block *sb)
> >  	}
> >  
> >  	ubifs_umount(c);
> > -	bdi_destroy(&c->bdi);
> >  	ubi_close_volume(c->ubi);
> >  	mutex_unlock(&c->umount_mutex);
> >  }
> > @@ -2019,29 +2018,23 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
> >  		goto out;
> >  	}
> >  
> > +	err = ubifs_parse_options(c, data, 0);
> > +	if (err)
> > +		goto out_close;
> > +
> >  	/*
> >  	 * UBIFS provides 'backing_dev_info' in order to disable read-ahead. For
> >  	 * UBIFS, I/O is not deferred, it is done immediately in readpage,
> >  	 * which means the user would have to wait not just for their own I/O
> >  	 * but the read-ahead I/O as well i.e. completely pointless.
> >  	 *
> > -	 * Read-ahead will be disabled because @c->bdi.ra_pages is 0.
> > +	 * Read-ahead will be disabled because @sb->s_bdi->ra_pages is 0.
> >  	 */
> > -	c->bdi.name = "ubifs",
> > -	c->bdi.capabilities = 0;
> 
> So ->capabilities is now zero by default since you use __GFP_ZERO in
> bdi_alloc().
> At least for UBIFS I'll add a comment on this, otherwise it is not so
> clear that UBIFS wants a BDI with no capabilities and how it achieves that.

OK, I've modified the comment to:

         * Read-ahead will be disabled because @sb->s_bdi->ra_pages is 0. Also
         * @sb->s_bdi->capabilities are initialized to 0 so there won't be any
         * writeback happening.
         */


> Acked-by: Richard Weinberger <richard@nod.at>

Thanks.

								Honza
Richard Weinberger Feb. 9, 2017, 2:56 p.m.
Am 09.02.2017 um 13:17 schrieb Jan Kara:
>> So ->capabilities is now zero by default since you use __GFP_ZERO in
>> bdi_alloc().
>> At least for UBIFS I'll add a comment on this, otherwise it is not so
>> clear that UBIFS wants a BDI with no capabilities and how it achieves that.
> 
> OK, I've modified the comment to:
> 
>          * Read-ahead will be disabled because @sb->s_bdi->ra_pages is 0. Also
>          * @sb->s_bdi->capabilities are initialized to 0 so there won't be any
>          * writeback happening.
>          */

Nice!

Thanks,
//richard

Patch hide | download patch | download mbox

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index e08aa04fc835..34810eb52b22 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1827,7 +1827,6 @@  static void ubifs_put_super(struct super_block *sb)
 	}
 
 	ubifs_umount(c);
-	bdi_destroy(&c->bdi);
 	ubi_close_volume(c->ubi);
 	mutex_unlock(&c->umount_mutex);
 }
@@ -2019,29 +2018,23 @@  static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 		goto out;
 	}
 
+	err = ubifs_parse_options(c, data, 0);
+	if (err)
+		goto out_close;
+
 	/*
 	 * UBIFS provides 'backing_dev_info' in order to disable read-ahead. For
 	 * UBIFS, I/O is not deferred, it is done immediately in readpage,
 	 * which means the user would have to wait not just for their own I/O
 	 * but the read-ahead I/O as well i.e. completely pointless.
 	 *
-	 * Read-ahead will be disabled because @c->bdi.ra_pages is 0.
+	 * Read-ahead will be disabled because @sb->s_bdi->ra_pages is 0.
 	 */
-	c->bdi.name = "ubifs",
-	c->bdi.capabilities = 0;
-	err  = bdi_init(&c->bdi);
+	err = super_setup_bdi_name(sb, "ubifs_%d_%d", c->vi.ubi_num,
+				   c->vi.vol_id);
 	if (err)
 		goto out_close;
-	err = bdi_register(&c->bdi, NULL, "ubifs_%d_%d",
-			   c->vi.ubi_num, c->vi.vol_id);
-	if (err)
-		goto out_bdi;
-
-	err = ubifs_parse_options(c, data, 0);
-	if (err)
-		goto out_bdi;
 
-	sb->s_bdi = &c->bdi;
 	sb->s_fs_info = c;
 	sb->s_magic = UBIFS_SUPER_MAGIC;
 	sb->s_blocksize = UBIFS_BLOCK_SIZE;
@@ -2080,8 +2073,6 @@  static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 	ubifs_umount(c);
 out_unlock:
 	mutex_unlock(&c->umount_mutex);
-out_bdi:
-	bdi_destroy(&c->bdi);
 out_close:
 	ubi_close_volume(c->ubi);
 out:
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index ca72382ce6cc..41b42a425b42 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -968,7 +968,6 @@  struct ubifs_debug_info;
  * struct ubifs_info - UBIFS file-system description data structure
  * (per-superblock).
  * @vfs_sb: VFS @struct super_block object
- * @bdi: backing device info object to make VFS happy and disable read-ahead
  *
  * @highest_inum: highest used inode number
  * @max_sqnum: current global sequence number
@@ -1216,7 +1215,6 @@  struct ubifs_debug_info;
  */
 struct ubifs_info {
 	struct super_block *vfs_sb;
-	struct backing_dev_info bdi;
 
 	ino_t highest_inum;
 	unsigned long long max_sqnum;
@@ -1457,7 +1455,6 @@  extern const struct inode_operations ubifs_file_inode_operations;
 extern const struct file_operations ubifs_dir_operations;
 extern const struct inode_operations ubifs_dir_inode_operations;
 extern const struct inode_operations ubifs_symlink_inode_operations;
-extern struct backing_dev_info ubifs_backing_dev_info;
 extern struct ubifs_compressor *ubifs_compressors[UBIFS_COMPR_TYPES_CNT];
 
 /* io.c */