Skip to contentSkip to conversation

cgroup, writeback: misc updates for cgroup writeback support

PATCH In review Tejun Heo 2 participants 6 messages Jun 16, 2015 – Jun 17, 2015

Subsystems (4)
lore
Share & tools
lore

Tejun Heo

author maintainer
#0 lore raw

PATCH v2 1/1 cgroup, writeback: misc updates for cgroup writeback support

Hello,

This is v2. The only change from the last take[L] is

* super_block->s_iflags added and MS_CGROUPWB replaced with SB_I_CGROUPWB as suggested by Christoph and Jan.

This patchset contains the following assorted updates for the cgroup writeback support.

0001-writeback-do-foreign-inode-detection-iff-cgroup-writ.patch 0002-vfs-writeback-replace-FS_CGROUP_WRITEBACK-with-SB_I_.patch 0003-writeback-blkio-add-documentation-for-cgroup-writeba.patch

0001 fixes a bug where clear FS_CGROUP_WRITEBACK flag didn't fully disable cgroup writeback support if the filesystem code uses wbc_init_bio() and wbc_account_io().

0002 replaces FS_CGROUP_WRITEBACK with SB_I_CGROUPWB so that cgroup writeback support can be enabled / disabled per superblock rather than filesystem type.

0003 updates blkio documentation with information on cgroup writeback support.

This patchset is on top of block/for-4.2/writeback and available in the following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-cgroup-writeback-updates

diffstat follows. Thanks.

 Documentation/cgroups/blkio-controller.txt |   83 +++++++++++++++++++++++++++--
 fs/ext2/super.c                            |    4 -
 fs/fs-writeback.c                          |   16 ++++-
 fs/namespace.c                             |    2 
 include/linux/backing-dev.h                |    2 
 include/linux/fs.h                         |    1 
 include/uapi/linux/fs.h                    |    1 
 7 files changed, 96 insertions(+), 13 deletions(-)

--
tejun

[L] http://lkml.kernel.org/g/1434146254-26220-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

author maintainer
#1 lore raw

PATCH 1/3 writeback: do foreign inode detection iff cgroup writeback is enabled

Currently, even when a filesystem doesn't set the FS_CGROUP_WRITEBACK flag, if the filesystem uses wbc_init_bio() and wbc_account_io(), the foreign inode detection and migration logic still ends up activating cgroup writeback which is unexpected. This patch ensures that the foreign inode detection logic stays disabled when inode_cgwb_enabled() is false by not associating writeback_control's with bdi_writeback's.

This also avoids unnecessary operations in wbc_init_bio(), wbc_account_io() and wbc_detach_inode() for filesystems which don't support cgroup writeback.

---
 fs/fs-writeback.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
fs/fs-writeback.c +13 -3
index f60de54..f0520bc 100644
@@ -513,6 +513,11 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id)
 void wbc_attach_and_unlock_inode(struct writeback_control *wbc,
 				 struct inode *inode)
 {
+	if (!inode_cgwb_enabled(inode)) {
+		spin_unlock(&inode->i_lock);
+		return;
+	}
+
 	wbc->wb = inode_to_wb(inode);
 	wbc->inode = inode;
 
@@ -575,11 +580,16 @@ void wbc_detach_inode(struct writeback_control *wbc)
 {
 	struct bdi_writeback *wb = wbc->wb;
 	struct inode *inode = wbc->inode;
-	u16 history = inode->i_wb_frn_history;
-	unsigned long avg_time = inode->i_wb_frn_avg_time;
-	unsigned long max_bytes, max_time;
+	unsigned long avg_time, max_bytes, max_time;
+	u16 history;
 	int max_id;
 
+	if (!wb)
+		return;
+
+	history = inode->i_wb_frn_history;
+	avg_time = inode->i_wb_frn_avg_time;
+
 	/* pick the winner of this round */
 	if (wbc->wb_bytes >= wbc->wb_lcand_bytes &&
 	    wbc->wb_bytes >= wbc->wb_tcand_bytes) {
Signature · 1 line
2.4.3
author maintainer
#2 lore raw

PATCH 2/3 vfs, writeback: replace FS_CGROUP_WRITEBACK with SB_I_CGROUPWB

In reply to Tejun Heo

FS_CGROUP_WRITEBACK indicates whether a file_system_type supports cgroup writeback; however, different super_blocks of the same file_system_type may or may not support cgroup writeback depending on filesystem options. This patch replaces FS_CGROUP_WRITEBACK with a per-super_block flag.

super_block->s_flags carries some internal flags in the high bits but it's exposd to userland through uapi header and running out of space anyway. This patch adds a new field super_block->s_iflags to carry kernel-internal flags. It is currently only used by the new SB_I_CGROUPWB flag whose concatenated and abbreviated name is for consistency with other super_block flags.

ext2_fill_super() is updated to set SB_I_CGROUPWB.

v2: Added super_block->s_iflags instead of stealing another high bit

    from sb->s_flags as suggested by Christoph and Jan.
Cc Alexander Viro <viro-RmSDqhL/[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Jan Kara <[email protected]>
---
 fs/ext2/super.c             | 3 ++-
 include/linux/backing-dev.h | 2 +-
 include/linux/fs.h          | 4 +++-
 3 files changed, 6 insertions(+), 3 deletions(-)
fs/ext2/super.c +2 -1
index 549219d..900e19c 100644
@@ -882,6 +882,7 @@ 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) ||
@@ -1543,7 +1544,7 @@ static struct file_system_type ext2_fs_type = {
 	.name		= "ext2",
 	.mount		= ext2_mount,
 	.kill_sb	= kill_block_super,
-	.fs_flags	= FS_REQUIRES_DEV | FS_CGROUP_WRITEBACK,
+	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("ext2");
 
include/linux/backing-dev.h +1 -1
index dfce808..a13181a 100644
@@ -260,7 +260,7 @@ static inline bool inode_cgwb_enabled(struct inode *inode)
 
 	return bdi_cap_account_dirty(bdi) &&
 		(bdi->capabilities & BDI_CAP_CGROUP_WRITEBACK) &&
-		(inode->i_sb->s_type->fs_flags & FS_CGROUP_WRITEBACK);
+		(inode->i_sb->s_iflags & SB_I_CGROUPWB);
 }
 
 /**
include/linux/fs.h +3 -1
index b5e1dcf..2c5e33a 100644
@@ -1241,6 +1241,8 @@ struct mm_struct;
 #define UMOUNT_NOFOLLOW	0x00000008	/* Don't follow symlink on umount */
 #define UMOUNT_UNUSED	0x80000000	/* Flag guaranteed to be unused */
 
+/* sb->s_iflags */
+#define SB_I_CGROUPWB	0x00000001	/* cgroup-aware writeback enabled */
 
 /* Possible states of 'frozen' field */
 enum {
@@ -1279,6 +1281,7 @@ struct super_block {
 	const struct quotactl_ops	*s_qcop;
 	const struct export_operations *s_export_op;
 	unsigned long		s_flags;
+	unsigned long		s_iflags;	/* internal SB_I_* flags */
 	unsigned long		s_magic;
 	struct dentry		*s_root;
 	struct rw_semaphore	s_umount;
@@ -1912,7 +1915,6 @@ struct file_system_type {
 #define FS_HAS_SUBTYPE		4
 #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
 #define FS_USERNS_DEV_MOUNT	16 /* A userns mount does not imply MNT_NODEV */
-#define FS_CGROUP_WRITEBACK	32	/* Supports cgroup-aware writeback */
 #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
 	struct dentry *(*mount) (struct file_system_type *, int,
 		       const char *, void *);
Signature · 1 line
2.4.3
author maintainer
#4 lore raw

PATCH 3/3 writeback, blkio: add documentation for cgroup writeback support

In reply to Tejun Heo

Update Documentation/cgroups/blkio-controller.txt to reflect the recently added cgroup writeback support.

---
 Documentation/cgroups/blkio-controller.txt | 83 ++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 5 deletions(-)
Documentation/cgroups/blkio-controller.txt +78 -5
index cd556b9..68b6a6a 100644
@@ -387,8 +387,81 @@ groups and put applications in that group which are not driving enough
 IO to keep disk busy. In that case set group_idle=0, and CFQ will not idle
 on individual groups and throughput should improve.
 
-What works
-==========
-- Currently only sync IO queues are support. All the buffered writes are
-  still system wide and not per group. Hence we will not see service
-  differentiation between buffered writes between groups.
+Writeback
+=========
+
+Page cache is dirtied through buffered writes and shared mmaps and
+written asynchronously to the backing filesystem by the writeback
+mechanism.  Writeback sits between the memory and IO domains and
+regulates the proportion of dirty memory by balancing dirtying and
+write IOs.
+
+On traditional cgroup hierarchies, relationships between different
+controllers cannot be established making it impossible for writeback
+to operate accounting for cgroup resource restrictions and all
+writeback IOs are attributed to the root cgroup.
+
+If both the blkio and memory controllers are used on the v2 hierarchy
+and the filesystem supports cgroup writeback, writeback operations
+correctly follow the resource restrictions imposed by both memory and
+blkio controllers.
+
+Writeback examines both system-wide and per-cgroup dirty memory status
+and enforces the more restrictive of the two.  Also, writeback control
+parameters which are absolute values - vm.dirty_bytes and
+vm.dirty_background_bytes - are distributed across cgroups according
+to their current writeback bandwidth.
+
+There's a peculiarity stemming from the discrepancy in ownership
+granularity between memory controller and writeback.  While memory
+controller tracks ownership per page, writeback operates on inode
+basis.  cgroup writeback bridges the gap by tracking ownership by
+inode but migrating ownership if too many foreign pages, pages which
+don't match the current inode ownership, have been encountered while
+writing back the inode.
+
+This is a conscious design choice as writeback operations are
+inherently tied to inodes making strictly following page ownership
+complicated and inefficient.  The only use case which suffers from
+this compromise is multiple cgroups concurrently dirtying disjoint
+regions of the same inode, which is an unlikely use case and decided
+to be unsupported.  Note that as memory controller assigns page
+ownership on the first use and doesn't update it until the page is
+released, even if cgroup writeback strictly follows page ownership,
+multiple cgroups dirtying overlapping areas wouldn't work as expected.
+In general, write-sharing an inode across multiple cgroups is not well
+supported.
+
+Filesystem support for cgroup writeback
+---------------------------------------
+
+A filesystem can make writeback IOs cgroup-aware by updating
+address_space_operations->writepage[s]() to annotate bio's using the
+following two functions.
+
+* wbc_init_bio(@wbc, @bio)
+
+  Should be called for each bio carrying writeback data and associates
+  the bio with the inode's owner cgroup.  Can be called anytime
+  between bio allocation and submission.
+
+* wbc_account_io(@wbc, @page, @bytes)
+
+  Should be called for each data segment being written out.  While
+  this function doesn't care exactly when it's called during the
+  writeback session, it's the easiest and most natural to call it as
+  data segments are added to a bio.
+
+With writeback bio's annotated, cgroup support can be enabled per
+super_block by setting MS_CGROUPWB in ->s_flags.  This allows for
+selective disabling of cgroup writeback support which is helpful when
+certain filesystem features, e.g. journaled data mode, are
+incompatible.
+
+wbc_init_bio() binds the specified bio to its cgroup.  Depending on
+the configuration, the bio may be executed at a lower priority and if
+the writeback session is holding shared resources, e.g. a journal
+entry, may lead to priority inversion.  There is no one easy solution
+for the problem.  Filesystems can try to work around specific problem
+cases by skipping wbc_init_bio() or using bio_associate_blkcg()
+directly.
Signature · 1 line
2.4.3
author maintainer
#5 lore raw
In reply to Tejun Heo
Quoted 18 lines · Tejun Heo

Hello,

This is v2. The only change from the last take[L] is

* super_block->s_iflags added and MS_CGROUPWB replaced with SB_I_CGROUPWB as suggested by Christoph and Jan.

This patchset contains the following assorted updates for the cgroup writeback support.

0001-writeback-do-foreign-inode-detection-iff-cgroup-writ.patch 0002-vfs-writeback-replace-FS_CGROUP_WRITEBACK-with-SB_I_.patch 0003-writeback-blkio-add-documentation-for-cgroup-writeba.patch

0001 fixes a bug where clear FS_CGROUP_WRITEBACK flag didn't fully disable cgroup writeback support if the filesystem code uses wbc_init_bio() and wbc_account_io().

Quoted 6 lines

0002 replaces FS_CGROUP_WRITEBACK with SB_I_CGROUPWB so that cgroup writeback support can be enabled / disabled per superblock rather than filesystem type.

0003 updates blkio documentation with information on cgroup writeback support.

Jens, this is late in the merge window but these three patches are low risk and would be nice to make available for -rc1 so that future filesystem cgroup writeback support doesn't have to pull in cgroup branch. What do you think?

Thanks.

Signature · 1 line
tejun