From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39149) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VfrPv-0006NR-W1 for qemu-devel@nongnu.org; Mon, 11 Nov 2013 08:20:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VfrPq-00015t-QC for qemu-devel@nongnu.org; Mon, 11 Nov 2013 08:20:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:14694) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VfrPq-00015Q-Hp for qemu-devel@nongnu.org; Mon, 11 Nov 2013 08:20:30 -0500 Date: Mon, 11 Nov 2013 14:20:20 +0100 From: Kevin Wolf Message-ID: <20131111132020.GD3046@dhcp-200-207.str.redhat.com> References: <1382609227-23989-1-git-send-email-pl@kamp.de> <1382609227-23989-11-git-send-email-pl@kamp.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1382609227-23989-11-git-send-email-pl@kamp.de> Subject: Re: [Qemu-devel] [PATCHv7 10/17] block: honour BlockLimits in bdrv_co_discard List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven Cc: pbonzini@redhat.com, ronniesahlberg@gmail.com, qemu-devel@nongnu.org, stefanha@redhat.com Am 24.10.2013 um 12:06 hat Peter Lieven geschrieben: > Reviewed-by: Eric Blake > Signed-off-by: Peter Lieven > --- > block.c | 37 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 36 insertions(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index 0c0b0ac..b28dd42 100644 > --- a/block.c > +++ b/block.c > @@ -4234,6 +4234,11 @@ static void coroutine_fn bdrv_discard_co_entry(void *opaque) > rwco->ret = bdrv_co_discard(rwco->bs, rwco->sector_num, rwco->nb_sectors); > } > > +/* if no limit is specified in the BlockLimits use a default > + * of 32768 512-byte sectors (16 MiB) per request. > + */ > +#define MAX_DISCARD_DEFAULT 32768 > + > int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, > int nb_sectors) > { > @@ -4255,7 +4260,37 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, > } > > if (bs->drv->bdrv_co_discard) { > - return bs->drv->bdrv_co_discard(bs, sector_num, nb_sectors); > + int max_discard = bs->bl.max_discard ? > + bs->bl.max_discard : MAX_DISCARD_DEFAULT; > + > + while (nb_sectors > 0) { > + int ret; > + int num = nb_sectors; > + > + /* align request */ > + if (bs->bl.discard_alignment && > + num >= bs->bl.discard_alignment && > + sector_num % bs->bl.discard_alignment) { > + if (num > bs->bl.discard_alignment) { > + num = bs->bl.discard_alignment; > + } > + num -= sector_num % bs->bl.discard_alignment; > + } > + > + /* limit request size */ > + if (num > max_discard) { > + num = max_discard; > + } > + > + ret = bs->drv->bdrv_co_discard(bs, sector_num, num); > + if (ret) { > + return ret; > + } > + > + sector_num += num; > + nb_sectors -= num; > + } > + return 0; > } else if (bs->drv->bdrv_aio_discard) { > BlockDriverAIOCB *acb; > CoroutineIOCompletion co = { You're only optimising drivers which have a .bdrv_co_discard() implementation, but not those with .bdrv_aio_discard(). Not very nice, and it would have been easy to avoid this by putting the loop around the whole if statement instead of inside the 'then' branch. Kevin