linux-kernel - Re: [PATCH v4 1/2] block, bfq: counted root group into 'num_groups_with_pending

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <0DD9CFF0-6110-497D-A352-9F37CADADC6B@linaro.org>
Date:   Wed, 20 Oct 2021 10:51:21 +0200
From:   Paolo Valente <paolo.valente@...aro.org>
To:     Yu Kuai <yukuai3@...wei.com>
Cc:     Jens Axboe <axboe@...nel.dk>,
        linux-block <linux-block@...r.kernel.org>,
        linux-kernel@...r.kernel.org, yi.zhang@...wei.com
Subject: Re: [PATCH v4 1/2] block, bfq: counted root group into
 'num_groups_with_pending_reqs'



> Il giorno 14 ott 2021, alle ore 03:45, Yu Kuai <yukuai3@...wei.com> ha scritto:
> 
> 'num_groups_with_pending_reqs' represents how many groups that are
> not root group and have pending requests. This patch also counted
> root group into 'num_groups_with_pending_reqs'.
> 
> Signed-off-by: Yu Kuai <yukuai3@...wei.com>
> ---
> block/bfq-iosched.c | 36 ++++++++++++++++++++++++++------
> block/bfq-wf2q.c    | 50 +++++++++++++++++++++++++++++++++------------
> 2 files changed, 67 insertions(+), 19 deletions(-)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index fec18118dc30..d251735383f7 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -852,6 +852,16 @@ void __bfq_weights_tree_remove(struct bfq_data *bfqd,
> 	bfq_put_queue(bfqq);
> }
> 
> +static inline void
> +bfq_clear_group_with_pending_reqs(struct bfq_data *bfqd,
> +				  struct bfq_entity *entity)
> +{
> +	if (entity->in_groups_with_pending_reqs) {
> +		entity->in_groups_with_pending_reqs = false;
> +		bfqd->num_groups_with_pending_reqs--;
> +	}
> +}
> +
> /*
>  * Invoke __bfq_weights_tree_remove on bfqq and decrement the number
>  * of active groups for each queue's inactive parent entity.
> @@ -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.

Thanks,
Paolo

> 		 * not performed immediately upon the deactivation of
> 		 * entity, but it is delayed to when it also happens
> 		 * that the first leaf descendant bfqq of entity gets
> @@ -889,10 +916,7 @@ void bfq_weights_tree_remove(struct bfq_data *bfqd,
> 		 * needed. See the comments on
> 		 * num_groups_with_pending_reqs for details.
> 		 */
> -		if (entity->in_groups_with_pending_reqs) {
> -			entity->in_groups_with_pending_reqs = false;
> -			bfqd->num_groups_with_pending_reqs--;
> -		}
> +		bfq_clear_group_with_pending_reqs(bfqd, entity);
> 	}
> 
> 	/*
> diff --git a/block/bfq-wf2q.c b/block/bfq-wf2q.c
> index b74cc0da118e..3e9e672aa302 100644
> --- a/block/bfq-wf2q.c
> +++ b/block/bfq-wf2q.c
> @@ -945,6 +945,42 @@ static void bfq_update_fin_time_enqueue(struct bfq_entity *entity,
> 
> 	bfq_active_insert(st, entity);
> }
> +#ifdef CONFIG_BFQ_GROUP_IOSCHED
> +static inline void
> +bfq_set_group_with_pending_reqs(struct bfq_data *bfqd,
> +				struct bfq_entity *entity)
> +{
> +	if (!entity->in_groups_with_pending_reqs) {
> +		entity->in_groups_with_pending_reqs = true;
> +		bfqd->num_groups_with_pending_reqs++;
> +	}
> +}
> +
> +static void bfq_update_groups_with_pending_reqs(struct bfq_entity *entity)
> +{
> +	struct bfq_queue *bfqq = bfq_entity_to_bfqq(entity);
> +
> +	if (bfqq) {
> +		/*
> +		 * If the entity represents bfq_queue, and the queue belongs to
> +		 * root cgroup.
> +		 */
> +		if (!entity->parent)
> +			bfq_set_group_with_pending_reqs(bfqq->bfqd,
> +				&bfqq->bfqd->root_group->entity);
> +	} else {
> +		/* If the entity represents bfq_group. */
> +		struct bfq_group *bfqg =
> +			container_of(entity, struct bfq_group, entity);
> +		struct bfq_data *bfqd = bfqg->bfqd;
> +
> +		bfq_set_group_with_pending_reqs(bfqd, entity);
> +	}
> +}
> +#else
> +#define bfq_update_groups_with_pending_reqs(entity) \
> +	do {} while (0)
> +#endif
> 
> /**
>  * __bfq_activate_entity - handle activation of entity.
> @@ -999,19 +1035,7 @@ static void __bfq_activate_entity(struct bfq_entity *entity,
> 		entity->on_st_or_in_serv = true;
> 	}
> 
> -#ifdef CONFIG_BFQ_GROUP_IOSCHED
> -	if (!bfq_entity_to_bfqq(entity)) { /* bfq_group */
> -		struct bfq_group *bfqg =
> -			container_of(entity, struct bfq_group, entity);
> -		struct bfq_data *bfqd = bfqg->bfqd;
> -
> -		if (!entity->in_groups_with_pending_reqs) {
> -			entity->in_groups_with_pending_reqs = true;
> -			bfqd->num_groups_with_pending_reqs++;
> -		}
> -	}
> -#endif
> -
> +	bfq_update_groups_with_pending_reqs(entity);
> 	bfq_update_fin_time_enqueue(entity, st, backshifted);
> }
> 
> -- 
> 2.31.1
>