* [PATCH 1/1] async_xor: It should add src_offs when dropping destination page
@ 2021-04-25 9:22 Xiao Ni
2021-04-26 6:32 ` Song Liu
0 siblings, 1 reply; 3+ messages in thread
From: Xiao Ni @ 2021-04-25 9:22 UTC (permalink / raw)
To: linux-raid; +Cc: song, dan.j.williams, yuyufen, ncroxon, heinzm
Now we support sharing one page if PAGE_SIZE is not equal stripe size. To support this,
it needs to support calculating xor value with different offsets for each r5dev. One
offset array is used to record those offsets.
In RMW mode, parity page is used as a source page. It sets ASYNC_TX_XOR_DROP_DST before
calculating xor value in ops_run_prexor5. So it needs to add src_list and src_offs at
the same time. Now it only needs src_list. So the xor value which is calculated is wrong.
It can cause data corruption problem.
I can reproduce this problem 100% on a POWER8 machine. The steps are:
mdadm -CR /dev/md0 -l5 -n3 /dev/sdb1 /dev/sdc1 /dev/sdd1 --size=3G
mkfs.xfs /dev/md0
mount /dev/md0 /mnt/test
mount: /mnt/test: mount(2) system call failed: Structure needs cleaning.
Fixes: 29bcff787 ("md/raid5: add new xor function to support different page offset")
Signed-off-by: Xiao Ni <xni@redhat.com>
---
crypto/async_tx/async_xor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/crypto/async_tx/async_xor.c b/crypto/async_tx/async_xor.c
index a057ecb..6cd7f70 100644
--- a/crypto/async_tx/async_xor.c
+++ b/crypto/async_tx/async_xor.c
@@ -233,6 +233,7 @@ async_xor_offs(struct page *dest, unsigned int offset,
if (submit->flags & ASYNC_TX_XOR_DROP_DST) {
src_cnt--;
src_list++;
+ src_offs++;
}
/* wait for any prerequisite operations */
--
2.7.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] async_xor: It should add src_offs when dropping destination page
2021-04-25 9:22 [PATCH 1/1] async_xor: It should add src_offs when dropping destination page Xiao Ni
@ 2021-04-26 6:32 ` Song Liu
2021-04-26 10:20 ` Xiao Ni
0 siblings, 1 reply; 3+ messages in thread
From: Song Liu @ 2021-04-26 6:32 UTC (permalink / raw)
To: Xiao Ni
Cc: linux-raid, Dan Williams, Yufen Yu, Nigel Croxon, Heinz Mauelshagen
On Sun, Apr 25, 2021 at 2:23 AM Xiao Ni <xni@redhat.com> wrote:
>
> Now we support sharing one page if PAGE_SIZE is not equal stripe size. To support this,
> it needs to support calculating xor value with different offsets for each r5dev. One
> offset array is used to record those offsets.
>
> In RMW mode, parity page is used as a source page. It sets ASYNC_TX_XOR_DROP_DST before
> calculating xor value in ops_run_prexor5. So it needs to add src_list and src_offs at
> the same time. Now it only needs src_list. So the xor value which is calculated is wrong.
> It can cause data corruption problem.
>
> I can reproduce this problem 100% on a POWER8 machine. The steps are:
> mdadm -CR /dev/md0 -l5 -n3 /dev/sdb1 /dev/sdc1 /dev/sdd1 --size=3G
> mkfs.xfs /dev/md0
> mount /dev/md0 /mnt/test
> mount: /mnt/test: mount(2) system call failed: Structure needs cleaning.
>
Thanks for the fix! Applied to md-next.
A few nits for future patches:
> Fixes: 29bcff787 ("md/raid5: add new xor function to support different page offset")
Please use "Fixes" with the first 12 characters of the hash: 29bcff787a25.
Also please run checkpatch.pl for the patch. In this one, it complains:
WARNING: Possible unwrapped commit description (prefer a maximum 75
chars per line)
Thanks,
Song
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
> crypto/async_tx/async_xor.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/crypto/async_tx/async_xor.c b/crypto/async_tx/async_xor.c
> index a057ecb..6cd7f70 100644
> --- a/crypto/async_tx/async_xor.c
> +++ b/crypto/async_tx/async_xor.c
> @@ -233,6 +233,7 @@ async_xor_offs(struct page *dest, unsigned int offset,
> if (submit->flags & ASYNC_TX_XOR_DROP_DST) {
> src_cnt--;
> src_list++;
> + src_offs++;
> }
>
> /* wait for any prerequisite operations */
> --
> 2.7.5
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] async_xor: It should add src_offs when dropping destination page
2021-04-26 6:32 ` Song Liu
@ 2021-04-26 10:20 ` Xiao Ni
0 siblings, 0 replies; 3+ messages in thread
From: Xiao Ni @ 2021-04-26 10:20 UTC (permalink / raw)
To: Song Liu
Cc: linux-raid, Dan Williams, Yufen Yu, Nigel Croxon, Heinz Mauelshagen
On 04/26/2021 02:32 PM, Song Liu wrote:
> On Sun, Apr 25, 2021 at 2:23 AM Xiao Ni <xni@redhat.com> wrote:
>> Now we support sharing one page if PAGE_SIZE is not equal stripe size. To support this,
>> it needs to support calculating xor value with different offsets for each r5dev. One
>> offset array is used to record those offsets.
>>
>> In RMW mode, parity page is used as a source page. It sets ASYNC_TX_XOR_DROP_DST before
>> calculating xor value in ops_run_prexor5. So it needs to add src_list and src_offs at
>> the same time. Now it only needs src_list. So the xor value which is calculated is wrong.
>> It can cause data corruption problem.
>>
>> I can reproduce this problem 100% on a POWER8 machine. The steps are:
>> mdadm -CR /dev/md0 -l5 -n3 /dev/sdb1 /dev/sdc1 /dev/sdd1 --size=3G
>> mkfs.xfs /dev/md0
>> mount /dev/md0 /mnt/test
>> mount: /mnt/test: mount(2) system call failed: Structure needs cleaning.
>>
> Thanks for the fix! Applied to md-next.
>
> A few nits for future patches:
>
>> Fixes: 29bcff787 ("md/raid5: add new xor function to support different page offset")
> Please use "Fixes" with the first 12 characters of the hash: 29bcff787a25.
>
> Also please run checkpatch.pl for the patch. In this one, it complains:
>
> WARNING: Possible unwrapped commit description (prefer a maximum 75
> chars per line)
>
> Thanks,
> Song
Thanks for reminding me. I'll do this next time.
Regards
Xiao
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-26 10:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-25 9:22 [PATCH 1/1] async_xor: It should add src_offs when dropping destination page Xiao Ni
2021-04-26 6:32 ` Song Liu
2021-04-26 10:20 ` Xiao Ni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.