Re: [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment into dma-helpers

[John Snow <jsnow@redhat.com>]

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 <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.
>>>
>>
>> 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 <jsnow@redhat.com>
>
> 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