Re: [PATCH BUGFIX V2 1/2] block, bfq: put async queues for root bfq groups too

[Guoqing Jiang]

On 01/09/2018 05:27 PM, Paolo Valente wrote:
> For each pair [device for which bfq is selected as I/O scheduler,
> group in blkio/io], bfq maintains a corresponding bfq group. Each such
> bfq group contains a set of async queues, with each async queue
> created on demand, i.e., when some I/O request arrives for it.  On
> creation, an async queue gets an extra reference, to make sure that
> the queue is not freed as long as its bfq group exists.  Accordingly,
> to allow the queue to be freed after the group exited, this extra
> reference must released on group exit.
>
> The above holds also for a bfq root group, i.e., for the bfq group
> corresponding to the root blkio/io root for a given device. Yet, by
> mistake, the references to the existing async queues of a root group
> are not released when the latter exits. This causes a memory leak when
> the instance of bfq for a given device exits. In a similar vein,
> bfqg_stats_xfer_dead is not executed for a root group.
>
> This commit fixes bfq_pd_offline so that the latter executes the above
> missing operations for a root group too.
>
> Reported-by: Holger HoffstÃtte <holger@xxxxxxxxxxxxxxxxxxxxxx>
> Reported-by: Guoqing Jiang <gqjiang@xxxxxxxx>
> Signed-off-by: Davide Ferrari <davideferrari8@xxxxxxxxx>
> Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
> ---
>   block/bfq-cgroup.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index da1525ec4c87..d819dc77fe65 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -775,10 +775,11 @@ static void bfq_pd_offline(struct blkg_policy_data *pd)
>   	unsigned long flags;
>   	int i;
>   
> +	spin_lock_irqsave(&bfqd->lock, flags);
> +
>   	if (!entity) /* root group */
> -		return;
> +		goto put_async_queues;
>   
> -	spin_lock_irqsave(&bfqd->lock, flags);
>   	/*
>   	 * Empty all service_trees belonging to this group before
>   	 * deactivating the group itself.
> @@ -809,6 +810,8 @@ static void bfq_pd_offline(struct blkg_policy_data *pd)
>   	}
>   
>   	__bfq_deactivate_entity(entity, false);
> +
> +put_async_queues:
>   	bfq_put_async_queues(bfqd, bfqg);
>   
>   	spin_unlock_irqrestore(&bfqd->lock, flags);

With this change, bfqg_stats_xfer_dead will be called even entity is not 
existed,
is it necessary? Thanks.

Regards,
Guoqing