From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48172) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaSwV-0002mg-HK for qemu-devel@nongnu.org; Tue, 24 Mar 2015 13:48:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaSwS-00070n-Ap for qemu-devel@nongnu.org; Tue, 24 Mar 2015 13:48:43 -0400 Received: from smtp3.mundo-r.com ([212.51.32.191]:44509 helo=smtp4.mundo-r.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaSwS-00070S-4N for qemu-devel@nongnu.org; Tue, 24 Mar 2015 13:48:40 -0400 Date: Tue, 24 Mar 2015 18:48:37 +0100 From: Alberto Garcia Message-ID: <20150324174837.GA27529@igalia.com> References: <20150324163145.GH1493@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150324163145.GH1493@stefanha-thinkpad.redhat.com> Subject: Re: [Qemu-devel] [PATCH 4/6] throttle: Add throttle group support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On Tue, Mar 24, 2015 at 04:31:45PM +0000, Stefan Hajnoczi wrote: > > + /* get next bs round in round robin style */ > > + token = throttle_group_next_bs(token); > > + while (token != start && > > + qemu_co_queue_empty(&token->throttled_reqs[is_write])) { > > It's not safe to access bs->throttled_reqs[]. There are many of > other places that access bs->throttled_reqs[] without holding > tg->lock (e.g. block.c). > > throttled_reqs needs to be protected by tg->lock in order for this > to work. Good catch, but I don't think that's the solution. throttled_reqs cannot be protected by that lock because it must release it before calling qemu_co_queue_wait(&bs->throttled_reqs[is_write]). Otherwise it will cause a deadlock. My assumption was that throttled_reqs would be protected by the BDS's AioContext lock, but I overlooked the fact that this part of the algorithm needs to access other BDSs' queues so we indeed have a problem. I will give it a thought to see what I can come up with. Since we only need to check whether other BDSs have pending requests maybe I can implement this with a flag. Thanks! Berto