[v5 2/2] raid6/altivec: Add vpermxor implementation for raid6 Q syndrome
Daniel Axtens
dja at axtens.net
Wed Aug 2 09:01:26 AEST 2017
Hi Matt,
> The raid6 Q syndrome check has been optimised using the vpermxor
> instruction.
Very much a nit, but normally we'd write the change that the patch makes
as a command: "Optimise the raid6 Q syndrome generation using the
vpermxor instruction" - see
https://www.kernel.org/doc/html/v4.11/process/submitting-patches.html#describe-your-changes
> +static void noinline raid6_vpermxor$#_gen_syndrome_real(int disks, size_t bytes,
> + void **ptrs)
> +{
> + u8 **dptr = (u8 **)ptrs;
> + u8 *p, *q;
> + int d, z, z0;
> + unative_t wp$$, wq$$, wd$$;
> +
> + z0 = disks - 3; /* Highest data disk */
> + p = dptr[z0+1]; /* XOR parity */
> + q = dptr[z0+2]; /* RS syndrome */
> +
> + for (d = 0; d < bytes; d += NSIZE*$#) {
> + wp$$ = wq$$ = *(unative_t *)&dptr[z0][d+$$*NSIZE];
> +
> + for (z = z0-1; z>=0; z--) {
> + wd$$ = *(unative_t *)&dptr[z][d+$$*NSIZE];
> + /* P syndrome */
> + wp$$ = vec_xor(wp$$, wd$$);
> +
> + /*Q syndrome */
> + asm("vpermxor %0,%1,%2,%3":"=v"(wq$$):"v"(gf_high), "v"(gf_low), "v"(wq$$));
Initially I thought "why can't we break this over 2 lines?" and then I
remembered that the awk script can't handle that. A space between /* and
Q would be good though.
> + wq$$ = vec_xor(wq$$, wd$$);
I generated some of the unrolled code and inspected it. It's non-trivial
to follow but that's justifiable, it's due to:
- the complex maths
- the unrolling process
- consistency with the altivec code, which I think is worth keeping
I am not sure how you could make it any easier to read, so I don't think
that should block its acceptance into the kernel.
I am confident that this code works correctly and as described.
Reviewed-by: Daniel Axtens <dja at axtens.net>
Regards,
Daniel
> --
> 2.9.3
More information about the Linuxppc-dev
mailing list