linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Valente <paolo.valente@linaro.org>
To: "yukuai (C)" <yukuai3@huawei.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	linux-block <linux-block@vger.kernel.org>,
	linux-kernel@vger.kernel.org, yi.zhang@huawei.com
Subject: Re: [PATCH v4 1/2] block, bfq: counted root group into 'num_groups_with_pending_reqs'
Date: Wed, 20 Oct 2021 11:29:29 +0200	[thread overview]
Message-ID: <2E8712BB-5BFB-4647-AE9A-B06E199500D7@linaro.org> (raw)
In-Reply-To: <1f89cece-a123-6190-bb72-d59035dac266@huawei.com>



> Il giorno 20 ott 2021, alle ore 11:20, yukuai (C) <yukuai3@huawei.com> ha scritto:
> 
> On 2021/10/20 16:51, Paolo Valente wrote:
> 
>>> @@ -860,9 +870,25 @@ void bfq_weights_tree_remove(struct bfq_data *bfqd,
>>> 			     struct bfq_queue *bfqq)
>>> {
>>> 	struct bfq_entity *entity = bfqq->entity.parent;
>>> +	struct bfq_sched_data *sd;
>>> +
>>> +	/*
>>> +	 * If the bfq queue is in root group, the decrement of
>>> +	 * num_groups_with_pending_reqs is performed immediately upon the
>>> +	 * deactivation of entity.
>>> +	 */
>>> +	if (!entity) {
>>> +		entity = &bfqd->root_group->entity;
>>> +		sd = entity->my_sched_data;
>>> +
>>> +		if (!sd->in_service_entity)
>>> +			bfq_clear_group_with_pending_reqs(bfqd, entity);
>>> +
>>> +		return;
>>> +	}
>>> 
>>> 	for_each_entity(entity) {
>>> -		struct bfq_sched_data *sd = entity->my_sched_data;
>>> +		sd = entity->my_sched_data;
>>> 
>>> 		if (sd->next_in_service || sd->in_service_entity) {
>>> 			/*
>>> @@ -880,7 +906,8 @@ void bfq_weights_tree_remove(struct bfq_data *bfqd,
>>> 		}
>>> 
>>> 		/*
>>> -		 * The decrement of num_groups_with_pending_reqs is
>>> +		 * If the bfq queue is not in root group,
>>> +		 * the decrement of num_groups_with_pending_reqs is
>> I'm sorry if I didn't notice this before, but why do you postpone the
>> decrement only for queues not in root group?  If I'm not missing
>> anything, the active (i.e., with pending reqs) state of the root group
>> is to be computed as that of ay other group.
> 
> Hi, Paolo
> 
> I thought if queue is in root group, then bfqq->entity.parent is NULL,
> and such case is handled above, which is separate from previous
> implementation for queues that are not in root group.
> 
> Is this the wrong way to handle root group?
> 

I think that, if we want to count also the root group among the active
ones, then the logic for tagging the root group as active must be the
same as the other groups. Or am I missing something?

Thanks,
Paolo

> Thanks,
> Kuai


  reply	other threads:[~2021-10-20  9:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-14  1:45 [PATCH v4 0/2] optimize the bfq queue idle judgment Yu Kuai
2021-10-14  1:45 ` [PATCH v4 1/2] block, bfq: counted root group into 'num_groups_with_pending_reqs' Yu Kuai
2021-10-20  8:51   ` Paolo Valente
2021-10-20  9:20     ` yukuai (C)
2021-10-20  9:29       ` Paolo Valente [this message]
2021-10-20  9:38         ` yukuai (C)
2021-10-20  9:43           ` Paolo Valente
2021-10-20 11:53             ` yukuai (C)
2021-10-14  1:45 ` [PATCH v4 2/2] block, bfq: do not idle if only one cgroup is activated Yu Kuai

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=2E8712BB-5BFB-4647-AE9A-B06E199500D7@linaro.org \
    --to=paolo.valente@linaro.org \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yi.zhang@huawei.com \
    --cc=yukuai3@huawei.com \
    /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).