[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 5/5] qcow2: Catch more unaligned write_zero i

From: Eric Blake
Subject: Re: [Qemu-block] [PATCH v3 5/5] qcow2: Catch more unaligned write_zero into zero cluster
Date: Thu, 2 Jun 2016 06:33:48 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 06/02/2016 04:14 AM, Kevin Wolf wrote:

>> If you prefer, I could have written '-tail % s->cluster_sectors', but as
>> % on a negative signed integer gives different results than what you get
>> for an unsigned number, I felt that & was nicer than % for making it
>> more obvious that I'm grabbing particular bits.
>> If you can think of any cleaner expression that represents the number of
>> sectors occurring after the tail until the next cluster boundary, I'm
>> game; the hardest part is that when tail is 0, we want the number passed
>> to is_zero_sectors() to also be 0, not s->cluster_sectors (so the naive
>> 's->cluster_sectors - tail' is wrong).
> The obvious one would be translating your English into C:
>     tail ? s->cluster_sectors - tail : 0

Would gcc optimize this into a bit operation rather than a branch?  If
not, that's a missed optimization bug that we should probably report
(that is, if gcc has enough information elsewhere that
s->cluster_sectors is a power of 2, since the bit operation optimization
depends on that fact).

> Another option which doesn't require an unsigned type would be
> (s->cluster_sectors - tail) % s->cluster_sectors.

That's the same thing as '-tail % s->cluster_sectors', since the
distributive rule applies to modulo arithmetic, and since 'a % a' is 0
for non-zero 'a'.  So you can argue I was just saving typing :)

> I'm okay with merging the "more interesting" version, though I must
> admit that I had to read it twice.

That's certainly your call as maintainer :)

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]