Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754752AbeAJBla (ORCPT + 1 other); Tue, 9 Jan 2018 20:41:30 -0500 Received: from smtp2.provo.novell.com ([137.65.250.81]:39571 "EHLO smtp2.provo.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753734AbeAJBl2 (ORCPT ); Tue, 9 Jan 2018 20:41:28 -0500 Subject: Re: [PATCH BUGFIX V2 1/2] block, bfq: put async queues for root bfq groups too To: Paolo Valente , Jens Axboe Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, ulf.hansson@linaro.org, broonie@kernel.org, linus.walleij@linaro.org, bfq-iosched@googlegroups.com, oleksandr@natalenko.name, 162996@studenti.unimore.it, Davide Ferrari References: <20180109092759.5454-1-paolo.valente@linaro.org> <20180109092759.5454-2-paolo.valente@linaro.org> From: Guoqing Jiang Message-ID: Date: Wed, 10 Jan 2018 09:41:14 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20180109092759.5454-2-paolo.valente@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: 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 > Reported-by: Guoqing Jiang > Signed-off-by: Davide Ferrari > Signed-off-by: Paolo Valente > --- > 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