From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52225)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mark.cave-ayland@ilande.co.uk>) id 1btgBH-00032y-Nc
	for qemu-devel@nongnu.org; Mon, 10 Oct 2016 15:24:12 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mark.cave-ayland@ilande.co.uk>) id 1btgBF-0006CV-R2
	for qemu-devel@nongnu.org; Mon, 10 Oct 2016 15:24:10 -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>
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-ID: <ab2a2471-d428-f82d-cd89-c6c5dcba44ed@ilande.co.uk>
Date: Mon, 10 Oct 2016 20:23:47 +0100
MIME-Version: 1.0
In-Reply-To: <97da94bb-e32c-8d9f-7309-ca4bef4bbe7c@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment
 into dma-helpers
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>, keith.busch@intel.com, kwolf@redhat.com, mreitz@redhat.com, jsnow@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org

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 <mark.cave-ayland@ilande.co.uk>
>> ---
> 
>> @@ -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 <eblake@redhat.com>

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.