Am 18.05.2011 11:18, schrieb Dmitry Konishchev:
> On 18.05.2011 11:57, Stefan Hajnoczi wrote:
>> Yes, optimizing is_not_zero() is good. The only additional thing I
>> suggest is adding a comment before the function to document the length
>> constraint.
>
> OK, fixed.
>
>
> On 18.05.2011 12:05, Kevin Wolf wrote:
>> A future bdrv_is_allocated() patch must make sure that the conversion
>> falls back to a simple is_not_zero() when a backing file is used.
>
> Thanks, I'll take this into account.
>
>
> Signed-off-by: Dmitry Konishchev <konishc...@gmail.com>
> ---
> qemu-img.c | 30 +++++++++++++++++++++++++++---
> 1 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index e825123..7665c2f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -496,14 +496,38 @@ static int img_commit(int argc, char **argv)
> return 0;
> }
>
> +/*
> + * Checks whether the sector is not a zero sector.
> + *
> + * Attention! The len must be a multiple of 4 * sizeof(long) due to
> + * restriction of optimizations in this function.
> + */
> static int is_not_zero(const uint8_t *sector, int len)
> {
> + /*
> + * Use long as the biggest available internal data type that fits
> into the
> + * CPU register and unroll the loop to smooth out the effect of memory
> + * latency.
> + */
> +
> int i;
> - len >>= 2;
> - for(i = 0;i < len; i++) {
> - if (((uint32_t *)sector)[i] != 0)
> + len /= sizeof(long);
> +
> + long d0;
> + long d1;
> + long d2;
> + long d3;
Please move the declarations to the start of the function.
I also would use a single line like "long d0, d1, d2, d3;", but that's
up to you.
> +
> + for(i = 0; i < len; i += 4) {
> + d0 = ((const long*) sector)[i + 0];
> + d1 = ((const long*) sector)[i + 1];
> + d2 = ((const long*) sector)[i + 2];
> + d3 = ((const long*) sector)[i + 3];
I would suggest to declare a const long* variable so that you don't have
to cast each time you use, but that's probably a matter of taste.
> +
> + if (d0 || d1 || d2 || d3)
> return 1;
Coding style requires braces here.
> }
> +
> return 0;
> }
Please make sure that your patch isn't line-wrapped when you send it for
inclusion. git send-email will do the right thing.
Kevin
