* [PATCH v2 2/4] btrfs-progs: volumes: Remove BUG_ON in raid56 write routine
2016-10-25 2:11 [PATCH v2 1/4] btrfs-progs: rename raid6.c to raid56.c Qu Wenruo
@ 2016-10-25 2:11 ` Qu Wenruo
2016-10-25 12:29 ` David Sterba
2016-10-25 2:11 ` [PATCH v2 3/4] btrfs-progs: raid56: Introduce new function to calculate raid5 parity or data stripe Qu Wenruo
2016-10-25 2:11 ` [PATCH v2 4/4] btrfs-progs: volumes: Use new raid5_gen_result to calculate raid5 parity Qu Wenruo
2 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2016-10-25 2:11 UTC (permalink / raw)
To: linux-btrfs, dsterba; +Cc: David Sterba
Remove various BUG_ON in raid56 write routine, including:
1) Memory allocation error
Old codes allocates memory when code needs new memory in a loop, and
catch the error using BUG_ON().
New codes allocates memory in a allocation loop first, if any failure
is caught, freeing already allocated memories then throw -ENOMEM.
2) Write error
Change BUG_ON() to correct return value.
3) Validation check
Same as write error.
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
changelog:
v2:
Fix error in calloc usage which leads to double free() on data
stripes.
Thanks David for pointing it out.
---
volumes.c | 89 +++++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 61 insertions(+), 28 deletions(-)
diff --git a/volumes.c b/volumes.c
index b9ca550..70d8940 100644
--- a/volumes.c
+++ b/volumes.c
@@ -2063,25 +2063,39 @@ static int rmw_eb(struct btrfs_fs_info *info,
return 0;
}
-static void split_eb_for_raid56(struct btrfs_fs_info *info,
- struct extent_buffer *orig_eb,
+static int split_eb_for_raid56(struct btrfs_fs_info *info,
+ struct extent_buffer *orig_eb,
struct extent_buffer **ebs,
u64 stripe_len, u64 *raid_map,
int num_stripes)
{
- struct extent_buffer *eb;
+ struct extent_buffer **tmp_ebs;
u64 start = orig_eb->start;
u64 this_eb_start;
int i;
- int ret;
+ int ret = 0;
+
+ tmp_ebs = calloc(num_stripes, sizeof(*tmp_ebs));
+ if (!tmp_ebs)
+ return -ENOMEM;
+ /* Alloc memory in a row for data stripes */
for (i = 0; i < num_stripes; i++) {
if (raid_map[i] >= BTRFS_RAID5_P_STRIPE)
break;
- eb = calloc(1, sizeof(struct extent_buffer) + stripe_len);
- if (!eb)
- BUG();
+ tmp_ebs[i] = calloc(1, sizeof(**tmp_ebs) + stripe_len);
+ if (!tmp_ebs[i]) {
+ ret = -ENOMEM;
+ goto clean_up;
+ }
+ }
+
+ for (i = 0; i < num_stripes; i++) {
+ struct extent_buffer *eb = tmp_ebs[i];
+
+ if (raid_map[i] >= BTRFS_RAID5_P_STRIPE)
+ break;
eb->start = raid_map[i];
eb->len = stripe_len;
@@ -2095,12 +2109,21 @@ static void split_eb_for_raid56(struct btrfs_fs_info *info,
if (start > this_eb_start ||
start + orig_eb->len < this_eb_start + stripe_len) {
ret = rmw_eb(info, eb, orig_eb);
- BUG_ON(ret);
+ if (ret)
+ goto clean_up;
} else {
- memcpy(eb->data, orig_eb->data + eb->start - start, stripe_len);
+ memcpy(eb->data, orig_eb->data + eb->start - start,
+ stripe_len);
}
ebs[i] = eb;
}
+ free(tmp_ebs);
+ return ret;
+clean_up:
+ for (i = 0; i < num_stripes; i++)
+ free(tmp_ebs[i]);
+ free(tmp_ebs);
+ return ret;
}
int write_raid56_with_parity(struct btrfs_fs_info *info,
@@ -2113,15 +2136,20 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
int j;
int ret;
int alloc_size = eb->len;
+ void **pointers;
- ebs = kmalloc(sizeof(*ebs) * multi->num_stripes, GFP_NOFS);
- BUG_ON(!ebs);
+ ebs = malloc(sizeof(*ebs) * multi->num_stripes);
+ pointers = malloc(sizeof(*pointers) * multi->num_stripes);
+ if (!ebs || !pointers)
+ return -ENOMEM;
if (stripe_len > alloc_size)
alloc_size = stripe_len;
- split_eb_for_raid56(info, eb, ebs, stripe_len, raid_map,
- multi->num_stripes);
+ ret = split_eb_for_raid56(info, eb, ebs, stripe_len, raid_map,
+ multi->num_stripes);
+ if (ret)
+ goto out;
for (i = 0; i < multi->num_stripes; i++) {
struct extent_buffer *new_eb;
@@ -2129,11 +2157,17 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
ebs[i]->dev_bytenr = multi->stripes[i].physical;
ebs[i]->fd = multi->stripes[i].dev->fd;
multi->stripes[i].dev->total_ios++;
- BUG_ON(ebs[i]->start != raid_map[i]);
+ if (ebs[i]->start != raid_map[i]) {
+ ret = -EINVAL;
+ goto out_free_split;
+ }
continue;
}
- new_eb = kmalloc(sizeof(*eb) + alloc_size, GFP_NOFS);
- BUG_ON(!new_eb);
+ new_eb = malloc(sizeof(*eb) + alloc_size);
+ if (!new_eb) {
+ ret = -ENOMEM;
+ goto out_free_split;
+ }
new_eb->dev_bytenr = multi->stripes[i].physical;
new_eb->fd = multi->stripes[i].dev->fd;
multi->stripes[i].dev->total_ios++;
@@ -2145,12 +2179,6 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
q_eb = new_eb;
}
if (q_eb) {
- void **pointers;
-
- pointers = kmalloc(sizeof(*pointers) * multi->num_stripes,
- GFP_NOFS);
- BUG_ON(!pointers);
-
ebs[multi->num_stripes - 2] = p_eb;
ebs[multi->num_stripes - 1] = q_eb;
@@ -2158,7 +2186,6 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
pointers[i] = ebs[i]->data;
raid6_gen_syndrome(multi->num_stripes, stripe_len, pointers);
- kfree(pointers);
} else {
ebs[multi->num_stripes - 1] = p_eb;
memcpy(p_eb->data, ebs[0]->data, stripe_len);
@@ -2177,12 +2204,18 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
for (i = 0; i < multi->num_stripes; i++) {
ret = write_extent_to_disk(ebs[i]);
- BUG_ON(ret);
- if (ebs[i] != eb)
- kfree(ebs[i]);
+ if (ret < 0)
+ goto out_free_split;
}
- kfree(ebs);
+out_free_split:
+ for (i = 0; i < multi->num_stripes; i++) {
+ if (ebs[i] != eb)
+ free(ebs[i]);
+ }
+out:
+ free(ebs);
+ free(pointers);
- return 0;
+ return ret;
}
--
2.10.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/4] btrfs-progs: raid56: Introduce new function to calculate raid5 parity or data stripe
2016-10-25 2:11 [PATCH v2 1/4] btrfs-progs: rename raid6.c to raid56.c Qu Wenruo
2016-10-25 2:11 ` [PATCH v2 2/4] btrfs-progs: volumes: Remove BUG_ON in raid56 write routine Qu Wenruo
@ 2016-10-25 2:11 ` Qu Wenruo
2016-10-25 2:11 ` [PATCH v2 4/4] btrfs-progs: volumes: Use new raid5_gen_result to calculate raid5 parity Qu Wenruo
2 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2016-10-25 2:11 UTC (permalink / raw)
To: linux-btrfs, dsterba; +Cc: David Sterba
Introduce new function raid5_gen_result() to calculate parity or data
stripe.
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
Changelog:
v2:
None
---
disk-io.h | 1 +
raid56.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+)
diff --git a/disk-io.h b/disk-io.h
index 8ab36aa..245626c 100644
--- a/disk-io.h
+++ b/disk-io.h
@@ -192,5 +192,6 @@ int write_and_map_eb(struct btrfs_trans_handle *trans, struct btrfs_root *root,
/* raid56.c */
void raid6_gen_syndrome(int disks, size_t bytes, void **ptrs);
+int raid5_gen_result(int nr_devs, size_t stripe_len, int dest, void **data);
#endif
diff --git a/raid56.c b/raid56.c
index 833df5f..8c79c45 100644
--- a/raid56.c
+++ b/raid56.c
@@ -26,6 +26,8 @@
#include "kerncompat.h"
#include "ctree.h"
#include "disk-io.h"
+#include "volumes.h"
+#include "utils.h"
/*
* This is the C data type to use
@@ -107,3 +109,64 @@ void raid6_gen_syndrome(int disks, size_t bytes, void **ptrs)
}
}
+static void xor_range(char *dst, const char*src, size_t size)
+{
+ /* Move to DWORD aligned */
+ while (size && ((unsigned long)dst & sizeof(unsigned long))) {
+ *dst++ ^= *src++;
+ size--;
+ }
+
+ /* DWORD aligned part */
+ while (size >= sizeof(unsigned long)) {
+ *(unsigned long *)dst ^= *(unsigned long *)src;
+ src += sizeof(unsigned long);
+ dst += sizeof(unsigned long);
+ size -= sizeof(unsigned long);
+ }
+ /* Remaining */
+ while (size) {
+ *dst++ ^= *src++;
+ size--;
+ }
+}
+
+/*
+ * Generate desired data/parity stripe for RAID5
+ *
+ * @nr_devs: Total number of devices, including parity
+ * @stripe_len: Stripe length
+ * @data: Data, with special layout:
+ * data[0]: Data stripe 0
+ * data[nr_devs-2]: Last data stripe
+ * data[nr_devs-1]: RAID5 parity
+ * @dest: To generate which data. should follow above data layout
+ */
+int raid5_gen_result(int nr_devs, size_t stripe_len, int dest, void **data)
+{
+ int i;
+ char *buf = data[dest];
+
+ /* Validation check */
+ if (stripe_len <= 0 || stripe_len != BTRFS_STRIPE_LEN) {
+ error("invalid parameter for %s", __func__);
+ return -EINVAL;
+ }
+
+ if (dest >= nr_devs || nr_devs < 2) {
+ error("invalid parameter for %s", __func__);
+ return -EINVAL;
+ }
+ /* Shortcut for 2 devs RAID5, which is just RAID1 */
+ if (nr_devs == 2) {
+ memcpy(data[dest], data[1 - dest], stripe_len);
+ return 0;
+ }
+ memset(buf, 0, stripe_len);
+ for (i = 0; i < nr_devs; i++) {
+ if (i == dest)
+ continue;
+ xor_range(buf, data[i], stripe_len);
+ }
+ return 0;
+}
--
2.10.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 4/4] btrfs-progs: volumes: Use new raid5_gen_result to calculate raid5 parity
2016-10-25 2:11 [PATCH v2 1/4] btrfs-progs: rename raid6.c to raid56.c Qu Wenruo
2016-10-25 2:11 ` [PATCH v2 2/4] btrfs-progs: volumes: Remove BUG_ON in raid56 write routine Qu Wenruo
2016-10-25 2:11 ` [PATCH v2 3/4] btrfs-progs: raid56: Introduce new function to calculate raid5 parity or data stripe Qu Wenruo
@ 2016-10-25 2:11 ` Qu Wenruo
2 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2016-10-25 2:11 UTC (permalink / raw)
To: linux-btrfs, dsterba; +Cc: David Sterba
Use thew raid5_gen_result() function to calculate raid5 parity.
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
Changelog:
v2:
None
---
volumes.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/volumes.c b/volumes.c
index 70d8940..6d7adef 100644
--- a/volumes.c
+++ b/volumes.c
@@ -2133,7 +2133,6 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
{
struct extent_buffer **ebs, *p_eb = NULL, *q_eb = NULL;
int i;
- int j;
int ret;
int alloc_size = eb->len;
void **pointers;
@@ -2188,18 +2187,12 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
raid6_gen_syndrome(multi->num_stripes, stripe_len, pointers);
} else {
ebs[multi->num_stripes - 1] = p_eb;
- memcpy(p_eb->data, ebs[0]->data, stripe_len);
- for (j = 1; j < multi->num_stripes - 1; j++) {
- for (i = 0; i < stripe_len; i += sizeof(u64)) {
- u64 p_eb_data;
- u64 ebs_data;
-
- p_eb_data = get_unaligned_64(p_eb->data + i);
- ebs_data = get_unaligned_64(ebs[j]->data + i);
- p_eb_data ^= ebs_data;
- put_unaligned_64(p_eb_data, p_eb->data + i);
- }
- }
+ for (i = 0; i < multi->num_stripes; i++)
+ pointers[i] = ebs[i]->data;
+ ret = raid5_gen_result(multi->num_stripes, stripe_len,
+ multi->num_stripes - 1, pointers);
+ if (ret < 0)
+ goto out_free_split;
}
for (i = 0; i < multi->num_stripes; i++) {
--
2.10.1
^ permalink raw reply related [flat|nested] 6+ messages in thread