On 11/12/20 6:40 AM, Peter Lieven wrote:
>> /*
>> - * Avoid that s->sector_next_status becomes unaligned to the source
>> - * request alignment and/or cluster size to avoid unnecessary read
>> - * cycles.
>> + * Avoid that s->sector_next_status becomes unaligned to the
>> + * source/destination request alignment and/or cluster size to avoid
>> + * unnecessary read/write cycles.
>> */
>> - tail = (sector_num - src_cur_offset + n) %
>> s->src_alignment[src_cur];
>> + alignment = MAX(s->src_alignment[src_cur], s->alignment);
>> + assert(is_power_of_2(alignment));
>> +
>> + tail = (sector_num - src_cur_offset + n) % alignment;
>> if (n > tail) {
>> n -= tail;
>> }
>
>
> I was also considering including the s->alignment when adding this chance.
> However, you need the least common multiple of both alignments, not the
> maximum, otherwise
>
> you might get misaligned to either source or destination.
>
>
> Why exactly do you need the power of two requirement?
The power of two requirement ensures that you h ave the least common
multiple of both alignments ;)
However, there ARE devices in practice that have advertised a
non-power-of-2 discard granularity, such as 15MiB (see commits 430b26a8,
63188c24). Which means you probably don't want to assert power-of-2,
and in turn need to worry about least common multiple.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
