Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1527947

Re: [PATCH V4 05/15] blk-throttle: add downgrade logic

From Tejun Heo <tj@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH V4 05/15] blk-throttle: add downgrade logic
Date 2016-11-22 22:30 +0100
Message-ID <sGqNA-4cd-13@gated-at.bofh.it> (permalink)
References <sDxVf-4Bm-3@gated-at.bofh.it> <sDxVf-4Bm-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hello,

On Mon, Nov 14, 2016 at 02:22:12PM -0800, Shaohua Li wrote:
> +static unsigned long tg_last_high_overflow_time(struct throtl_grp *tg)
> +{
> +	struct throtl_service_queue *parent_sq;
> +	struct throtl_grp *parent = tg;
> +	unsigned long ret = __tg_last_high_overflow_time(tg);
> +
> +	while (true) {
> +		parent_sq = parent->service_queue.parent_sq;
> +		parent = sq_to_tg(parent_sq);
> +		if (!parent)
> +			break;
> +		if (((parent->bps[READ][LIMIT_HIGH] != -1 &&
> +		      parent->bps[READ][LIMIT_HIGH] >=
> +		       tg->bps[READ][LIMIT_HIGH]) ||
> +		     (parent->bps[READ][LIMIT_HIGH] == -1 &&
> +		      parent->bps[READ][LIMIT_MAX] >=
> +		       tg->bps[READ][LIMIT_HIGH])) &&
> +		    ((parent->bps[WRITE][LIMIT_HIGH] != -1 &&
> +		      parent->bps[WRITE][LIMIT_HIGH] >=
> +		       tg->bps[WRITE][LIMIT_HIGH]) ||
> +		     (parent->bps[WRITE][LIMIT_HIGH] == -1 &&
> +		      parent->bps[WRITE][LIMIT_MAX] >=
> +		       tg->bps[WRITE][LIMIT_HIGH])) &&
> +		    ((parent->iops[READ][LIMIT_HIGH] != -1 &&
> +		      parent->iops[READ][LIMIT_HIGH] >=
> +		       tg->iops[READ][LIMIT_HIGH]) ||
> +		     (parent->iops[READ][LIMIT_HIGH] == -1 &&
> +		      parent->iops[READ][LIMIT_MAX] >=
> +		       tg->iops[READ][LIMIT_HIGH])) &&
> +		    ((parent->iops[WRITE][LIMIT_HIGH] != -1 &&
> +		      parent->iops[WRITE][LIMIT_HIGH] >=
> +		       tg->iops[WRITE][LIMIT_HIGH]) ||
> +		     (parent->iops[WRITE][LIMIT_HIGH] == -1 &&
> +		      parent->iops[WRITE][LIMIT_MAX] >=
> +		       tg->iops[WRITE][LIMIT_HIGH])))
> +			break;
> +		if (time_after(__tg_last_high_overflow_time(parent), ret))
> +			ret = __tg_last_high_overflow_time(parent);
> +	}
> +	return ret;
> +}

Heh, I'm not really following the upgrade/downgrade logic.  I'm having
trouble understanding two things.

1. A cgroup and its high and max limits don't have much to do with
   other cgroups and their limits.  I don't get how the choice between
   high and max limits can be a td-wide state.

2. Comparing parent's and child's limits and saying that either can be
   ignored because one is higher than the other isn't correct.  A
   parent's limit doesn't apply to each child separately.  It has to
   be aggregated.  e.g. you can ignore a parent's setting if the sum
   of all children's limits is smaller than the parent's but then
   again there could still be a lower limit higher up the tree, so
   they would still have to be examined.

Thanks.

-- 
tejun

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH V4 05/15] blk-throttle: add downgrade logic Shaohua Li <shli@fb.com> - 2016-11-14 23:30 +0100
  Re: [PATCH V4 05/15] blk-throttle: add downgrade logic Tejun Heo <tj@kernel.org> - 2016-11-22 22:30 +0100
    Re: [PATCH V4 05/15] blk-throttle: add downgrade logic Tejun Heo <tj@kernel.org> - 2016-11-22 22:50 +0100

csiph-web