On Mon, May 18, 2009 at 06:00:22PM -0700, Dan Williams wrote:

+static void makedata(void)
+{
+	int i, j;
+
+	for (i = 0; i < NDISKS; i++) {

i < NDISKS - 2 would be sufficient.

+static char disk_type(int d)
+{
+	switch (d) {
+	case NDISKS-2:
+		return 'P';
+	case NDISKS-1:
+		return 'Q';
+	default:
+		return 'D';
+	}
+}

I like this function very much because "if (disk_type(faila) == 'Q')"
is so much more readable than "if (faila == num_disks - 1)". It's a
pity that we only have it in this test module ;)

+/* Recover two failed blocks. */
+static void raid6_dual_recov(int disks, size_t bytes, int faila, int failb, struct page **ptrs)

As disks is always NDISKS, the disks parameter could be removed.

+{
+	struct async_submit_ctl submit;
+	addr_conv_t addr_conv[NDISKS];
+	struct completion cmp;
+	struct dma_async_tx_descriptor *tx = NULL;
+	enum sum_check_flags result = ~0;
+	bool dataq = false;
+
+	if (faila > failb)
+		swap(faila, failb);
+
+	init_async_submit(&submit, 0, NULL, NULL, NULL, addr_conv);
+	if (failb == disks-1) {

if (disk_type(failb) == 'Q'). Similar for the other tests in this function.

+	/* Generate assumed good syndrome */
+	init_completion(&cmp);
+	init_async_submit(&submit, ASYNC_TX_ACK, NULL, callback, &cmp, addr_conv);
+	async_gen_syndrome(dataptrs, 0, NDISKS, PAGE_SIZE, &submit);

How hard would it be to also test the fallback code for the synchronous
paths? AFAICS this test module wouldn't notice errors in the fallback
logic.

+config ASYNC_RAID6_TEST
+	tristate "Self test for hardware accelerated raid6 recovery"
+	depends on MD_RAID6_PQ
+	select ASYNC_RAID6_RECOV
+	---help---
+	  This is a one-shot self test that permutes through the
+	  recovery of all the possible two disk failure scenarios for a
+	  N-disk array.  Recovery is performed with the asynchronous

Currently N is 16. So s/N-disk/raid6 perhaps?

Regards
Andre
-- 
The only person who always got his work done by Friday was Robinson Crusoe