[v5 2/2] raid6/altivec: Add vpermxor implementation for raid6 Q syndrome

[Daniel Axtens]

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