Re: [PATCH 1/3] cgroup, writeback: replace SB_I_CGROUPWB with per-inode S_CGROUPWB

[Jan Kara]

On Mon 09-10-17 14:29:09, Tejun Heo wrote:
> Currently, filesystem can indiate cgroup writeback support per
> superblock; however, depending on the filesystem, especially if inodes
> are used to carry metadata, it can be useful to indicate cgroup
> writeback support per inode.
> 
> This patch replaces the superblock flag SB_I_CGROUPWB with per-inode
> S_CGROUPWB, so that cgroup writeback can be enabled selectively.
> 
> * block_dev sets the new flag in bdget() when initializing new inode.
> 
> * ext2/4 set the new flag in ext?_set_inode_flags() function.
> 
> * btrfs sets the new flag in btrfs_update_iflags() function.  Note
>   that this automatically excludes btree_inode which doesn't use
>   btrfs_update_iflags() during initialization.  This is an intended
>   behavior change.
> 
> Signed-off-by: Tejun Heo 
> Cc: Jan Kara 
> Cc: Jens Axboe 
> Cc: Chris Mason 
> Cc: Josef Bacik 
> Cc: linux-btrfs@vger.kernel.org
> Cc: "Theodore Ts'o" 
> Cc: Andreas Dilger 
> Cc: linux-e...@vger.kernel.org

...

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 31db875..344f12b 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4591,8 +4591,11 @@ void ext4_set_inode_flags(struct inode *inode)
>   !ext4_should_journal_data(inode) && !ext4_has_inline_data(inode) &&
>   !ext4_encrypted_inode(inode))
>   new_fl |= S_DAX;
> + if (test_opt(inode->i_sb, DATA_FLAGS) != EXT4_MOUNT_JOURNAL_DATA)
> + new_fl |= S_CGROUPWB;

Use ext4_should_journal_data(inode) here? Ext4 can be journalling data also
because of per-inode flag.

Otherwise the patch looks good. You can add:

Reviewed-by: Jan Kara 

Honza
-- 
Jan Kara 
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] cgroup, writeback: replace SB_I_CGROUPWB with per-inode S_CGROUPWB

[Tejun Heo]

Currently, filesystem can indiate cgroup writeback support per
superblock; however, depending on the filesystem, especially if inodes
are used to carry metadata, it can be useful to indicate cgroup
writeback support per inode.

This patch replaces the superblock flag SB_I_CGROUPWB with per-inode
S_CGROUPWB, so that cgroup writeback can be enabled selectively.

* block_dev sets the new flag in bdget() when initializing new inode.

* ext2/4 set the new flag in ext?_set_inode_flags() function.

* btrfs sets the new flag in btrfs_update_iflags() function.  Note
  that this automatically excludes btree_inode which doesn't use
  btrfs_update_iflags() during initialization.  This is an intended
  behavior change.

Signed-off-by: Tejun Heo 
Cc: Jan Kara 
Cc: Jens Axboe 
Cc: Chris Mason 
Cc: Josef Bacik 
Cc: linux-btrfs@vger.kernel.org
Cc: "Theodore Ts'o" 
Cc: Andreas Dilger 
Cc: linux-e...@vger.kernel.org
---
 fs/block_dev.c  | 3 +--
 fs/btrfs/ioctl.c| 4 +++-
 fs/btrfs/super.c| 1 -
 fs/ext2/inode.c | 3 ++-
 fs/ext2/super.c | 1 -
 fs/ext4/inode.c | 5 -
 fs/ext4/super.c | 2 --
 include/linux/backing-dev.h | 2 +-
 include/linux/fs.h  | 3 ++-
 9 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 93d088f..ff9c282 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -800,8 +800,6 @@ static struct dentry *bd_mount(struct file_system_type 
*fs_type,
 {
struct dentry *dent;
dent = mount_pseudo(fs_type, "bdev:", _sops, NULL, BDEVFS_MAGIC);
-   if (!IS_ERR(dent))
-   dent->d_sb->s_iflags |= SB_I_CGROUPWB;
return dent;
 }
 
@@ -893,6 +891,7 @@ struct block_device *bdget(dev_t dev)
inode->i_mode = S_IFBLK;
inode->i_rdev = dev;
inode->i_bdev = bdev;
+   inode->i_flags |= S_CGROUPWB;
inode->i_data.a_ops = _blk_aops;
mapping_set_gfp_mask(>i_data, GFP_USER);
spin_lock(_lock);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 6c7a49f..117cc63 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -150,9 +150,11 @@ void btrfs_update_iflags(struct inode *inode)
new_fl |= S_NOATIME;
if (ip->flags & BTRFS_INODE_DIRSYNC)
new_fl |= S_DIRSYNC;
+   new_fl |= S_CGROUPWB;
 
set_mask_bits(>i_flags,
- S_SYNC | S_APPEND | S_IMMUTABLE | S_NOATIME | S_DIRSYNC,
+ S_SYNC | S_APPEND | S_IMMUTABLE | S_NOATIME | S_DIRSYNC |
+ S_CGROUPWB,
  new_fl);
 }
 
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 35a128a..46a1488 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1136,7 +1136,6 @@ static int btrfs_fill_super(struct super_block *sb,
sb->s_flags |= MS_POSIXACL;
 #endif
sb->s_flags |= MS_I_VERSION;
-   sb->s_iflags |= SB_I_CGROUPWB;
 
err = super_setup_bdi(sb);
if (err) {
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 4dca6f3..3c5d822 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1372,7 +1372,7 @@ void ext2_set_inode_flags(struct inode *inode)
unsigned int flags = EXT2_I(inode)->i_flags;
 
inode->i_flags &= ~(S_SYNC | S_APPEND | S_IMMUTABLE | S_NOATIME |
-   S_DIRSYNC | S_DAX);
+   S_DIRSYNC | S_DAX | S_CGROUPWB);
if (flags & EXT2_SYNC_FL)
inode->i_flags |= S_SYNC;
if (flags & EXT2_APPEND_FL)
@@ -1385,6 +1385,7 @@ void ext2_set_inode_flags(struct inode *inode)
inode->i_flags |= S_DIRSYNC;
if (test_opt(inode->i_sb, DAX) && S_ISREG(inode->i_mode))
inode->i_flags |= S_DAX;
+   inode->i_flags |= S_CGROUPWB;
 }
 
 struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 1458706..e6ba669e 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -922,7 +922,6 @@ static int ext2_fill_super(struct super_block *sb, void 
*data, int silent)
sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
((EXT2_SB(sb)->s_mount_opt & EXT2_MOUNT_POSIX_ACL) ?
 MS_POSIXACL : 0);
-   sb->s_iflags |= SB_I_CGROUPWB;
 
if (le32_to_cpu(es->s_rev_level) == EXT2_GOOD_OLD_REV &&
(EXT2_HAS_COMPAT_FEATURE(sb, ~0U) ||
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 31db875..344f12b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4591,8 +4591,11 @@ void ext4_set_inode_flags(struct inode *inode)
!ext4_should_journal_data(inode) && !ext4_has_inline_data(inode) &&
!ext4_encrypted_inode(inode))
new_fl |= S_DAX;
+   if (test_opt(inode->i_sb, DATA_FLAGS) !=