Messages in this thread | | | From | Jeff Moyer <> | Subject | Re: [PATCH] blkio-throttle: Avoid calling blkiocg_lookup_group() for root group | Date | Mon, 31 Jan 2011 14:20:57 -0500 |
| |
Vivek Goyal <vgoyal@redhat.com> writes:
> On Mon, Jan 17, 2011 at 04:56:06PM -0500, Vivek Goyal wrote: >> o Jeff Moyer was doing some testing on a RAM backed disk and >> blkiocg_lookup_group() showed up high overhead after memcpy(). Similarly >> somebody else reported that blkiocg_lookup_group() is eating 6% extra >> cpu. Though looking at the code I can't think why the overhead of >> this function is so high. One thing is that it is called with very high >> frequency (once for every IO). >> >> o For lot of folks blkio controller will be compiled in but they might >> not have actually created cgroups. Hence optimize the case of root >> cgroup where we can avoid calling blkiocg_lookup_group() if IO is happening >> in root group (common case). >> >> Reported-by: Jeff Moyer <jmoyer@redhat.com> >> Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > > Hi Jens, > > Do you have any concerns regarding this patch?
Acked-by: Jeff Moyer <jmoyer@redhat.com>
Jens, this is a pretty easy performance regression fixup. I think it should be pulled in sooner rather than later. We've also witnessed this slowdown on big performance testing rigs, so it's not just a ramdisk issue.
Cheers, Jeff
>> --- >> block/blk-throttle.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> Index: linux-2.6/block/blk-throttle.c >> =================================================================== >> --- linux-2.6.orig/block/blk-throttle.c 2011-01-17 16:23:37.041280712 -0500 >> +++ linux-2.6/block/blk-throttle.c 2011-01-17 16:36:09.362940975 -0500 >> @@ -168,7 +168,15 @@ static struct throtl_grp * throtl_find_a >> * tree of blkg (instead of traversing through hash list all >> * the time. >> */ >> - tg = tg_of_blkg(blkiocg_lookup_group(blkcg, key)); >> + >> + /* >> + * This is the common case when there are no blkio cgroups. >> + * Avoid lookup in this case >> + */ >> + if (blkcg == &blkio_root_cgroup) >> + tg = &td->root_tg; >> + else >> + tg = tg_of_blkg(blkiocg_lookup_group(blkcg, key)); >> >> /* Fill in device details for root group */ >> if (tg && !tg->blkg.dev && bdi->dev && dev_name(bdi->dev)) { >>
| |