From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35079) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buM0w-000872-KI for qemu-devel@nongnu.org; Wed, 12 Oct 2016 12:04:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buM0v-0000j7-Hl for qemu-devel@nongnu.org; Wed, 12 Oct 2016 12:04:18 -0400 References: <1476031419-6805-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1476031419-6805-2-git-send-email-mark.cave-ayland@ilande.co.uk> <97da94bb-e32c-8d9f-7309-ca4bef4bbe7c@redhat.com> <3fa3f1f8-2b7e-8b1f-1272-ff3b3647e0d3@redhat.com> <20161012102207.GF5544@noname.redhat.com> From: John Snow Message-ID: <2c7681e5-4b4b-263a-b2e3-6236f951a2d7@redhat.com> Date: Wed, 12 Oct 2016 12:04:06 -0400 MIME-Version: 1.0 In-Reply-To: <20161012102207.GF5544@noname.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment into dma-helpers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Mark Cave-Ayland , Eric Blake , keith.busch@intel.com, mreitz@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org On 10/12/2016 06:22 AM, Kevin Wolf wrote: > Am 11.10.2016 um 17:47 hat John Snow geschrieben: >> On 10/10/2016 03:23 PM, Mark Cave-Ayland wrote: >>> On 10/10/16 17:34, Eric Blake wrote: >>> >>>> On 10/09/2016 11:43 AM, Mark Cave-Ayland wrote: >>>>> The hard-coded default alignment is BDRV_SECTOR_SIZE, however this is not >>>>> necessarily the case for all platforms. Use this as the default alignment for >>>>> all current callers. >>>>> >>>>> Signed-off-by: Mark Cave-Ayland >>>>> --- >>>> >>>>> @@ -160,8 +161,8 @@ static void dma_blk_cb(void *opaque, int ret) >>>>> return; >>>>> } >>>>> >>>>> - if (dbs->iov.size & ~BDRV_SECTOR_MASK) { >>>>> - qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK); >>>>> + if (dbs->iov.size & (dbs->align - 1)) { >>>>> + qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & (dbs->align - 1)); >>>> >>>> Would it be any smarter to use osdep.h's QEMU_IS_ALIGNED(dbs->iov.size, >>>> dbs->align) and QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align)? >>>> Semantically it is the same, but the macros make it obvious what the >>>> bit-twiddling is doing. >>>> >>>> Unless you think that needs a tweak, >>>> Reviewed-by: Eric Blake >>> >>> I can't say I feel too strongly about it since there are plenty of other >>> examples of this style in the codebase, so I'm happy to go with whatever >>> John/Paolo are most happy with. >>> >>> >>> ATB, >>> >>> Mark. >>> >> >> I can't pretend I am consistent, but when in doubt use the macro. >> Not worth a respin IMO, but I think this falls out of my >> jurisdiction :) >> >> Acked-by: John Snow > > dma-helpers.c is officially unmaintained, and as the other patch is > clearly IDE, I think the series should go through your tree. > > Kevin > Oh! I was under the impression that Paolo had the dma-helpers. My mistake. I will test and stage this. --js