Re: [PATCHSET v2] cgroup, writeback, btrfs: make sure btrfs issues metadata IOs from the root cgroup

[Jan Kara]

On Wed 29-11-17 13:38:26, Chris Mason wrote:
> On 11/29/2017 12:05 PM, Tejun Heo wrote:
> >On Wed, Nov 29, 2017 at 09:03:30AM -0800, Tejun Heo wrote:
> >>Hello,
> >>
> >>On Wed, Nov 29, 2017 at 05:56:08PM +0100, Jan Kara wrote:
> >>>What has happened with this patch set?
> >>
> >>No idea.  cc'ing Chris directly.  Chris, if the patchset looks good,
> >>can you please route them through the btrfs tree?
> >
> >lol looking at the patchset again, I'm not sure that's obviously the
> >right tree.  It can either be cgroup, block or btrfs.  If no one
> >objects, I'll just route them through cgroup.
> 
> We'll have to coordinate a bit during the next merge window but I don't have
> a problem with these going in through cgroup.  Dave does this sound good to
> you?

Also I was wondering about another thing: How does this play with Josef's
series for metadata writeback (Metadata specific accouting and dirty
writeout)? Would the per-inode selection of cgroup writeback still be
needed when Josef's series is applied since metadata writeback then won't
be associated with any particular mapping anymore?

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

Re: [PATCHSET v2] cgroup, writeback, btrfs: make sure btrfs issues metadata IOs from the root cgroup

[Chris Mason]

On 11/30/2017 12:23 PM, David Sterba wrote:

On Wed, Nov 29, 2017 at 01:38:26PM -0500, Chris Mason wrote:

On 11/29/2017 12:05 PM, Tejun Heo wrote:

On Wed, Nov 29, 2017 at 09:03:30AM -0800, Tejun Heo wrote:

Hello,

On Wed, Nov 29, 2017 at 05:56:08PM +0100, Jan Kara wrote:

What has happened with this patch set?


No idea.  cc'ing Chris directly.  Chris, if the patchset looks good,
can you please route them through the btrfs tree?


lol looking at the patchset again, I'm not sure that's obviously the
right tree.  It can either be cgroup, block or btrfs.  If no one
objects, I'll just route them through cgroup.


We'll have to coordinate a bit during the next merge window but I don't
have a problem with these going in through cgroup.  Dave does this sound
good to you?


There are only minor changes to btrfs code so cgroup tree would be
better.


I'd like to include my patch to do all crcs inline (instead of handing
off to helper threads) when io controls are in place.  By the merge
window we should have some good data on how much it's all helping.


Are there any problems in sight if the inline crc and cgroup chnanges go
separately? I assume there's a runtime dependency, not a code
dependency, so it could be sorted by the right merge order.



The feature is just more useful with the inline crcs.  Without them we 
end up with kworkers doing both high and low prio submissions and it all 
boils down to the speed of the lowest priority.


-chris

--
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

Re: [PATCHSET v2] cgroup, writeback, btrfs: make sure btrfs issues metadata IOs from the root cgroup

[David Sterba]

On Wed, Nov 29, 2017 at 01:38:26PM -0500, Chris Mason wrote:
> On 11/29/2017 12:05 PM, Tejun Heo wrote:
> > On Wed, Nov 29, 2017 at 09:03:30AM -0800, Tejun Heo wrote:
> >> Hello,
> >>
> >> On Wed, Nov 29, 2017 at 05:56:08PM +0100, Jan Kara wrote:
> >>> What has happened with this patch set?
> >>
> >> No idea.  cc'ing Chris directly.  Chris, if the patchset looks good,
> >> can you please route them through the btrfs tree?
> > 
> > lol looking at the patchset again, I'm not sure that's obviously the
> > right tree.  It can either be cgroup, block or btrfs.  If no one
> > objects, I'll just route them through cgroup.
> 
> We'll have to coordinate a bit during the next merge window but I don't 
> have a problem with these going in through cgroup.  Dave does this sound 
> good to you?

There are only minor changes to btrfs code so cgroup tree would be
better.

> I'd like to include my patch to do all crcs inline (instead of handing 
> off to helper threads) when io controls are in place.  By the merge 
> window we should have some good data on how much it's all helping.

Are there any problems in sight if the inline crc and cgroup chnanges go
separately? I assume there's a runtime dependency, not a code
dependency, so it could be sorted by the right merge order.
--
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

Re: [PATCHSET v2] cgroup, writeback, btrfs: make sure btrfs issues metadata IOs from the root cgroup

[Chris Mason]

On 11/29/2017 12:05 PM, Tejun Heo wrote:

On Wed, Nov 29, 2017 at 09:03:30AM -0800, Tejun Heo wrote:

Hello,

On Wed, Nov 29, 2017 at 05:56:08PM +0100, Jan Kara wrote:

What has happened with this patch set?


No idea.  cc'ing Chris directly.  Chris, if the patchset looks good,
can you please route them through the btrfs tree?


lol looking at the patchset again, I'm not sure that's obviously the
right tree.  It can either be cgroup, block or btrfs.  If no one
objects, I'll just route them through cgroup.


We'll have to coordinate a bit during the next merge window but I don't 
have a problem with these going in through cgroup.  Dave does this sound 
good to you?


I'd like to include my patch to do all crcs inline (instead of handing 
off to helper threads) when io controls are in place.  By the merge 
window we should have some good data on how much it's all helping.


-chris

--
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

Re: [PATCHSET v2] cgroup, writeback, btrfs: make sure btrfs issues metadata IOs from the root cgroup

[Tejun Heo]

On Wed, Nov 29, 2017 at 09:03:30AM -0800, Tejun Heo wrote:
> Hello,
> 
> On Wed, Nov 29, 2017 at 05:56:08PM +0100, Jan Kara wrote:
> > What has happened with this patch set?
> 
> No idea.  cc'ing Chris directly.  Chris, if the patchset looks good,
> can you please route them through the btrfs tree?

lol looking at the patchset again, I'm not sure that's obviously the
right tree.  It can either be cgroup, block or btrfs.  If no one
objects, I'll just route them through cgroup.

Thanks.

-- 
tejun
--
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

Re: [PATCHSET v2] cgroup, writeback, btrfs: make sure btrfs issues metadata IOs from the root cgroup

[Tejun Heo]

Hello,

On Wed, Nov 29, 2017 at 05:56:08PM +0100, Jan Kara wrote:
> What has happened with this patch set?

No idea.  cc'ing Chris directly.  Chris, if the patchset looks good,
can you please route them through the btrfs tree?

Thanks.

-- 
tejun
--
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

Re: [PATCHSET v2] cgroup, writeback, btrfs: make sure btrfs issues metadata IOs from the root cgroup

[Jan Kara]

Hi Tejun,

What has happened with this patch set?

Honza

On Tue 10-10-17 08:54:36, Tejun Heo wrote:
> Changes from the last version are
> 
> * blkcg_root_css exported to fix build breakage on modular btrfs.
> 
> * Use ext4_should_journal_data() test instead of
>   EXT4_MOUNT_JOURNAL_DATA.
> 
> * Separated out create_bh_bio() and used it to implement
>   submit_bh_blkcg_css() as suggested by Jan.
> 
> btrfs has different ways to issue metadata IOs and may end up issuing
> metadata or otherwise shared IOs from a non-root cgroup, which can
> lead to priority inversion and ineffective IO isolation.
> 
> This patchset makes sure that btrfs issues all metadata and shared IOs
> from the root cgroup by exempting btree_inodes from cgroup writeback
> and explicitly associating shared IOs with the root cgroup.
> 
> This patchset containst he following three patches
> 
>  [PATCH 1/5] blkcg: export blkcg_root_css
>  [PATCH 2/5] cgroup, writeback: replace SB_I_CGROUPWB with per-inode
>  [PATCH 3/5] buffer_head: separate out create_bh_bio() from
>  [PATCH 4/5] cgroup, buffer_head: implement submit_bh_blkcg_css()
>  [PATCH 5/5] btrfs: ensure that metadata and flush are issued from the
> 
> and is also available in the following git branch
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git 
> review-cgroup-btrfs-metadata-v2
> 
> diffstat follows.  Thanks.
> 
>  block/blk-cgroup.c  |1 +
>  fs/block_dev.c  |3 +--
>  fs/btrfs/check-integrity.c  |2 +-
>  fs/btrfs/disk-io.c  |4 
>  fs/btrfs/ioctl.c|6 +-
>  fs/btrfs/super.c|1 -
>  fs/buffer.c |   42 ++
>  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/buffer_head.h |3 +++
>  include/linux/fs.h  |3 ++-
>  14 files changed, 58 insertions(+), 20 deletions(-)
> 
> --
> tejun
-- 
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

[PATCHSET v2] cgroup, writeback, btrfs: make sure btrfs issues metadata IOs from the root cgroup

[Tejun Heo]

Hello,

Changes from the last version are

* blkcg_root_css exported to fix build breakage on modular btrfs.

* Use ext4_should_journal_data() test instead of
  EXT4_MOUNT_JOURNAL_DATA.

* Separated out create_bh_bio() and used it to implement
  submit_bh_blkcg_css() as suggested by Jan.

btrfs has different ways to issue metadata IOs and may end up issuing
metadata or otherwise shared IOs from a non-root cgroup, which can
lead to priority inversion and ineffective IO isolation.

This patchset makes sure that btrfs issues all metadata and shared IOs
from the root cgroup by exempting btree_inodes from cgroup writeback
and explicitly associating shared IOs with the root cgroup.

This patchset containst he following three patches

 [PATCH 1/5] blkcg: export blkcg_root_css
 [PATCH 2/5] cgroup, writeback: replace SB_I_CGROUPWB with per-inode
 [PATCH 3/5] buffer_head: separate out create_bh_bio() from
 [PATCH 4/5] cgroup, buffer_head: implement submit_bh_blkcg_css()
 [PATCH 5/5] btrfs: ensure that metadata and flush are issued from the

and is also available in the following git branch

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git 
review-cgroup-btrfs-metadata-v2

diffstat follows.  Thanks.

 block/blk-cgroup.c  |1 +
 fs/block_dev.c  |3 +--
 fs/btrfs/check-integrity.c  |2 +-
 fs/btrfs/disk-io.c  |4 
 fs/btrfs/ioctl.c|6 +-
 fs/btrfs/super.c|1 -
 fs/buffer.c |   42 ++
 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/buffer_head.h |3 +++
 include/linux/fs.h  |3 ++-
 14 files changed, 58 insertions(+), 20 deletions(-)

--
tejun
--
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