From: Mike Snitzer <snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Nikolay Borisov <kernel-6AxghH7DbtA@public.gmane.org>,
Chris Friesen
<chris.friesen-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org
Subject: [PATCH] block: transfer source bio's cgroup tags to clone via bio_associate_blkcg() (was: Re: blkio cgroups controller doesn't work with LVM?)
Date: Wed, 2 Mar 2016 12:56:56 -0500 [thread overview]
Message-ID: <20160302175656.GA59991@redhat.com> (raw)
In-Reply-To: <20160302160649.GB29826-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Wed, Mar 02 2016 at 11:06P -0500,
Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Hello,
>
> On Thu, Feb 25, 2016 at 09:53:14AM -0500, Mike Snitzer wrote:
> > Right, LVM created devices are bio-based DM devices in the kernel.
> > bio-based block devices do _not_ have an IO scheduler. Their underlying
> > request-based device does.
>
> dm devices are not the actual resource source, so I don't think it'd
> work too well to put io controllers on them (can't really do things
> like proportional control without owning the queue).
>
> > I'm not well-versed on the top-level cgroup interface and how it maps to
> > associated resources that are established in the kernel. But it could
> > be that the configuration of blkio cgroup against a bio-based LVM device
> > needs to be passed through to the underlying request-based device
> > (e.g. /dev/sda4 in Chris's case)?
> >
> > I'm also wondering whether the latest cgroup work that Tejun has just
> > finished (afaik to support buffered IO in the IO controller) will afford
> > us a more meaningful reason to work to make cgroups' blkio controller
> > actually work with bio-based devices like LVM's DM devices?
> >
> > I'm very much open to advice on how to proceed with investigating this
> > integration work. Tejun, Vivek, anyone else: if you have advice on next
> > steps for DM on this front _please_ yell, thanks!
>
> I think the only thing necessary is dm transferring bio cgroup tags to
> the bio's that it ends up passing down the stack. Please take a look
> at fs/btrfs/extent_io.c::btrfs_bio_clone() for an example. We
> probably should introduce a wrapper for this so that each site doesn't
> need to ifdef it.
>
> Thanks.
OK, I think this should do it. Nikolay and/or others can you test this
patch using blkio cgroups controller with LVM devices and report back?
From: Mike Snitzer <snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Wed, 2 Mar 2016 12:37:39 -0500
Subject: [PATCH] block: transfer source bio's cgroup tags to clone via bio_associate_blkcg()
Move btrfs_bio_clone()'s support for transferring a source bio's cgroup
tags to a clone into both bio_clone_bioset() and __bio_clone_fast().
The former is used by btrfs (MD and blk-core also use it via bio_split).
The latter is used by both DM and bcache.
This should enable the blkio cgroups controller to work with all
stacking bio-based block devices.
Reported-by: Nikolay Borisov <kernel-6AxghH7DbtA@public.gmane.org>
Suggested-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Mike Snitzer <snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
block/bio.c | 10 ++++++++++
fs/btrfs/extent_io.c | 6 ------
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index cf75915..25812be 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -584,6 +584,11 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src)
bio->bi_rw = bio_src->bi_rw;
bio->bi_iter = bio_src->bi_iter;
bio->bi_io_vec = bio_src->bi_io_vec;
+
+#ifdef CONFIG_BLK_CGROUP
+ if (bio_src->bi_css)
+ bio_associate_blkcg(bio, bio_src->bi_css);
+#endif
}
EXPORT_SYMBOL(__bio_clone_fast);
@@ -689,6 +694,11 @@ integrity_clone:
}
}
+#ifdef CONFIG_BLK_CGROUP
+ if (bio_src->bi_css)
+ bio_associate_blkcg(bio, bio_src->bi_css);
+#endif
+
return bio;
}
EXPORT_SYMBOL(bio_clone_bioset);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 392592d..8abc330 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2691,12 +2691,6 @@ struct bio *btrfs_bio_clone(struct bio *bio, gfp_t gfp_mask)
btrfs_bio->csum = NULL;
btrfs_bio->csum_allocated = NULL;
btrfs_bio->end_io = NULL;
-
-#ifdef CONFIG_BLK_CGROUP
- /* FIXME, put this into bio_clone_bioset */
- if (bio->bi_css)
- bio_associate_blkcg(new, bio->bi_css);
-#endif
}
return new;
}
--
2.5.4 (Apple Git-61)
next prev parent reply other threads:[~2016-03-02 17:56 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-24 18:12 blkio cgroups controller doesn't work with LVM? Chris Friesen
[not found] ` <56CDF283.9010802-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
2016-02-25 7:48 ` Nikolay Borisov
[not found] ` <56CEB1BC.4000005-6AxghH7DbtA@public.gmane.org>
2016-02-25 14:53 ` Mike Snitzer
[not found] ` <20160225145314.GA20699-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-02-25 15:15 ` Chris Friesen
2016-02-26 16:42 ` [dm-devel] " Vivek Goyal
[not found] ` <20160226164228.GA24711-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-02-26 16:45 ` Vivek Goyal
2016-03-02 16:06 ` Tejun Heo
[not found] ` <20160302160649.GB29826-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2016-03-02 17:56 ` Mike Snitzer [this message]
[not found] ` <20160302175656.GA59991-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-03-02 18:03 ` [PATCH] block: transfer source bio's cgroup tags to clone via bio_associate_blkcg() (was: Re: blkio cgroups controller doesn't work with LVM?) Nikolay Borisov
[not found] ` <CAJFSNy6hni1-NWDs0z=Hq223=DfcjsNPoAb7GRAGEPCUXh4Q9w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-02 18:05 ` Mike Snitzer
2016-03-02 19:18 ` [PATCH] " Vivek Goyal
2016-03-02 19:59 ` Nikolay Borisov
2016-03-02 20:10 ` Vivek Goyal
2016-03-02 20:19 ` Nikolay Borisov
[not found] ` <CAJFSNy6MUGr8E3RNw6hFiskcaG4m8EGdqMkQXVh1LGq-yZCjBg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-02 20:45 ` Vivek Goyal
[not found] ` <20160302201016.GE3476-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-03-02 20:34 ` [PATCH] block: transfer source bio's cgroup tags to clone via bio_associate_blkcg() Chris Friesen
[not found] ` <56D74E6A.9050708-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
2016-03-02 21:04 ` Vivek Goyal
[not found] ` <20160302210405.GG3476-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-03-02 21:19 ` Vivek Goyal
2016-03-14 15:08 ` Nikolay Borisov
[not found] ` <56E6D3D3.4070104-6AxghH7DbtA@public.gmane.org>
2016-03-14 15:31 ` Nikolay Borisov
[not found] ` <56E6D95B.3030904-/eCPMmvKun9pLGFMi4vTTA@public.gmane.org>
2016-03-14 19:49 ` Mike Snitzer
[not found] ` <20160314194912.GA6975-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-03-14 22:08 ` Nikolay Borisov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160302175656.GA59991@redhat.com \
--to=snitzer-h+wxahxf7alqt0dzr+alfa@public.gmane.org \
--cc=axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org \
--cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=chris.friesen-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org \
--cc=dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=kernel-6AxghH7DbtA@public.gmane.org \
--cc=linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).