Re: [PATCH v2 11/11] async_tx: raid6 recovery self test

[Andre Noll <maan@systemlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]