From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45167) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaRkB-0006wV-Qg for qemu-devel@nongnu.org; Tue, 24 Mar 2015 12:31:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaRk5-0003lF-Mm for qemu-devel@nongnu.org; Tue, 24 Mar 2015 12:31:55 -0400 Received: from mail-wi0-x236.google.com ([2a00:1450:400c:c05::236]:34035) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaRk5-0003l9-Fy for qemu-devel@nongnu.org; Tue, 24 Mar 2015 12:31:49 -0400 Received: by wibg7 with SMTP id g7so55934436wib.1 for ; Tue, 24 Mar 2015 09:31:48 -0700 (PDT) Date: Tue, 24 Mar 2015 16:31:45 +0000 From: Stefan Hajnoczi Message-ID: <20150324163145.GH1493@stefanha-thinkpad.redhat.com> References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="KscVNZbUup0vZz0f" Content-Disposition: inline In-Reply-To: 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: Alberto Garcia Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi --KscVNZbUup0vZz0f Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Mar 10, 2015 at 05:30:48PM +0200, Alberto Garcia wrote: > +/* Return the next BlockDriverState in the round-robin sequence with > + * pending I/O requests. > + * > + * @bs: the current BlockDriverState > + * @is_write: the type of operation (read/write) > + * @ret: the next BlockDriverState with pending requests, or bs > + * if there is none. > + */ > +static BlockDriverState *next_throttle_token(BlockDriverState *bs, > + bool is_write) > +{ > + ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); > + BlockDriverState *token, *start; > + > + start = token = tg->tokens[is_write]; > + > + /* 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. > +/* Check if an I/O request needs to be throttled, wait and set a timer > + * if necessary, and schedule the next request using a round robin > + * algorithm. > + * > + * @bs: the current BlockDriverState > + * @bytes: the number of bytes for this I/O > + * @is_write: the type of operation (read/write) > + */ > +void throttle_group_io_limits_intercept(BlockDriverState *bs, > + unsigned int bytes, > + bool is_write) This function must be called from coroutine context because it uses qemu_co_queue_wait() and may yield. Please mark the function with coroutine_fn (see include/block/coroutine.h). This serves as documentation. > +{ > + bool must_wait; > + BlockDriverState *token; > + > + ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); > + qemu_mutex_lock(&tg->lock); > + > + /* First we check if this I/O has to be throttled. */ > + token = next_throttle_token(bs, is_write); > + must_wait = throttle_group_schedule_timer(token, is_write); > + > + /* Wait if there's a timer set or queued requests of this type */ > + if (must_wait || !qemu_co_queue_empty(&bs->throttled_reqs[is_write])) { > + qemu_mutex_unlock(&tg->lock); > + qemu_co_queue_wait(&bs->throttled_reqs[is_write]); Here bs->throttled_reqs[] is used without holding tg->lock. It can race with token->throttled_reqs[] users. --KscVNZbUup0vZz0f Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVEZFxAAoJEJykq7OBq3PI1pYH/jGvvK9S78PTEKMImJWqdg0Q t6x3U2tNKW+ws4M2vu8pwIyj9IRBmog/XdlAVXbvJ4xl2ma2LiqFuzffVoGR2qEg nifs1gAU4ra8+GmPtY0v12juXqI2ezCoPKr9Xp0jxtOeca8KS0WQge1SfPgesVKX GUML8/Zd0RIjAiEVoMOaZsIU3V0Ecvalnh/fYK+umoSv+2mYitZyYUxnys9roZ8I 5D6bmI9BldxBfq5T7BZTSE5BFmNrwPkgaUiwyFAc+wIBrNy/KTkrRzzemV6n1Oo2 8TzkG2+KphklksUIpxkNfRBYK2K/9R5bXB87YsffoNQH0hgIJwKj+eSzsnnfHDo= =xWar -----END PGP SIGNATURE----- --KscVNZbUup0vZz0f--