On 1/18/07, Yuri Tikhonov <yur_t@xxxxxxx> wrote:
Hello, Dan.
Hello.
It seems there is a bug in your 06.11.30 raid acceleration patch-set. I tried to run the Linux s/w RAID-5 driver patched with your 06.11.30 patch-set and
found that it fails during
write operations when the RAID-5 array consists of 6 or more number
of drives (I tested up
to 8 drives). For 5 and less number of drives everything works as
expected. There are no
such problems with your 06.09.12 set of patches. Do you have any
assumptions about the
reasons of this fault?
Yes, sorry, there were bugs in the synchronous path around handling > MAX_XOR_BLOCKS that I have fixed for the next rev of the patches. I'll be releasing them shortly, but attached is a patch to address the issue you are seeing.
The kernel I used was 2.6.19, your 06.11.30 patch-set was applied without any warnings/errors. Here is the kernel Oops report: Oops: kernel access of bad area, sig: 11 [#1] NIP: C014F980 LR: C014FD0C CTR: 00000080 REGS: eee49d40 TRAP: 0300 Not tainted (2.6.19-g0726acdc-dirty) MSR: 00029000 <EE,ME> CR: 44002042 XER: 20000000 DAR: 17970004, DSISR: 00000000 TASK = eed5a7d0[280] 'md0_raid5_ops/0' THREAD: eee48000 GPR00: 0000007F EEE49DF0 EED5A7D0 00000080 EEDFC000 00000000 19D70000 17870000 GPR08: 00001000 C02B0000 EEDFC000 C014F950 EEDFC000 30000000 C015B8D8 17970000 GPR16: C08AC180 C02B0000 EEE0CB48 0000003A 0000000C 00001000 00000000 00000001 GPR24: 00000000 C015B8D8 00000004 0000003A EEDFC000 00000004 19D70000 17870000 NIP [C014F980] xor_32regs_4+0x30/0x158 LR [C014FD0C] xor_block+0xc4/0x12c Call Trace: [EEE49E40] [EEE49E58] 0xeee49e58 [EEE49E50] [C014EFAC] async_xor+0x134/0x200 [EEE49EB0] [C015A960] ops_run_postxor+0xf8/0x198 [EEE49F00] [C0162458] raid5_run_ops+0x8dc/0x994 [EEE49F50] [C0029F7C] run_workqueue+0xa4/0x118 [EEE49F70] [C002A198] worker_thread+0xf8/0x13c [EEE49FC0] [C002E20C] kthread+0xf8/0x100 [EEE49FF0] [C0003DA0] kernel_thread+0x44/0x60 Instruction dump: 5463d97e 7c601b78 3400ffff 9421ffb0 bde1000c 7c6903a6 7c8c2378 7caf2b78 7cde3378 7cff3b78 41800124 80ac0000 <82ef0004> 82cf0008 82af000c 828f0010 Regards, Yuri.
Thanks for testing the patches. Regards, Dan
diff --git a/drivers/dma/async_tx.c b/drivers/dma/async_tx.c index d918cc3..eee208d 100644 --- a/drivers/dma/async_tx.c +++ b/drivers/dma/async_tx.c @@ -324,9 +324,6 @@ async_tx_find_channel(struct dma_async_tx_descriptor *depend_tx, } #endif -#define to_iop_adma_chan(chan) container_of(chan, struct iop_adma_chan, common) -#define tx_to_iop_adma_slot(tx) container_of(tx, struct iop_adma_desc_slot, async_tx) - static inline void async_tx_submit(struct dma_chan *chan, struct dma_async_tx_descriptor *tx, enum async_tx_flags flags, struct dma_async_tx_descriptor *depend_tx, @@ -423,17 +420,12 @@ do_sync_xor(struct page *dest, struct page **src_list, unsigned int offset, dma_async_tx_callback callback, void *callback_param) { void *_dest; - int start_idx, i; + int i; PRINTK("%s: len: %u\n", __FUNCTION__, len); /* reuse the 'src_list' array to convert to buffer pointers */ - if (flags & ASYNC_TX_XOR_DROP_DST) - start_idx = 1; - else - start_idx = 0; - - for (i = start_idx; i < src_cnt; i++) + for (i = 0; i < src_cnt; i++) src_list[i] = (struct page *) (page_address(src_list[i]) + offset); @@ -443,8 +435,8 @@ do_sync_xor(struct page *dest, struct page **src_list, unsigned int offset, if (flags & ASYNC_TX_XOR_ZERO_DST) memset(_dest, 0, len); - xor_block(src_cnt - start_idx, len, _dest, - (void **) &src_list[start_idx]); + xor_block(src_cnt, len, _dest, + (void **) src_list); sync_epilog(flags, depend_tx, callback, callback_param); } @@ -514,7 +506,15 @@ async_xor(struct page *dest, struct page **src_list, unsigned int offset, goto xor_sync; } else { /* run the xor synchronously */ xor_sync: - /* process up to 'max_xor_blocks' sources */ + /* in the sync case the dest is an implied source + * (assumes the dest is at the src_off index) + */ + if (flags & ASYNC_TX_XOR_DROP_DST) { + src_cnt--; + src_off++; + } + + /* process up to 'MAX_XOR_BLOCKS' sources */ xor_src_cnt = min(src_cnt, (unsigned int) MAX_XOR_BLOCKS); /* if we are submitting additional xors @@ -540,9 +540,9 @@ xor_sync: __FUNCTION__); } - do_sync_xor(dest, &src_list[src_off], offset, src_cnt, - len, local_flags, depend_tx, _callback, - _callback_param); + do_sync_xor(dest, &src_list[src_off], offset, + xor_src_cnt, len, local_flags, depend_tx, + _callback, _callback_param); } /* the previous tx is hidden from the client, @@ -556,13 +556,15 @@ xor_sync: if (src_cnt > xor_src_cnt) { /* drop completed sources */ src_cnt -= xor_src_cnt; + src_off += xor_src_cnt; /* unconditionally preserve the destination */ flags &= ~ASYNC_TX_XOR_ZERO_DST; - /* use the intermediate result a source */ - src_off = xor_src_cnt - 1; - src_list[src_off] = dest; + /* use the intermediate result a source, but remember + * it's dropped, because it's implied, in the sync case + */ + src_list[--src_off] = dest; src_cnt++; flags |= ASYNC_TX_XOR_DROP_DST; } else