Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
Andrew Morton wrote: On Thu, 22 Feb 2007 13:39:56 +1100 Neil Brown [EMAIL PROTECTED] wrote: I must right code that Andrew can read. That's write. But more importantly, things that people can immediately see and understand help reduce the possibility of mistakes. Now and in the future. If we did all loops like that, then it'd be the the best way to do it in new code, because people's eyes and brains are locked into that idiom and we just don't have to think about it when we see it. I have done lots of loops like that and understood it immediately. Nice, short, _clear_ and no - a loop that counts down instead of up is not difficult at all. Testing i-- instead of i = 0 is also something I consider trivial, even though I don't code that much. If this is among the worst you see, then the kernel source must be in great shape ;-) Helge Hafting - To unsubscribe from this list: send the line unsubscribe linux-raid in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
On Tue, 20 Feb 2007 17:35:16 +1100
NeilBrown [EMAIL PROTECTED] wrote:
+ for (i = conf-raid_disks ; i-- ; ) {
That statement should be dragged out, shot, stomped on then ceremonially
incinerated.
What's wrong with doing
for (i = 0; i conf-raid_disks; i++) {
in a manner which can be understood without alcoholic fortification?
ho hum.
-
To unsubscribe from this list: send the line unsubscribe linux-raid in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
From: Andrew Morton
Newsgroups: gmane.linux.raid,gmane.linux.kernel
Subject: Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
Date: Wed, 21 Feb 2007 14:48:06 -0800
Hallo.
On Tue, 20 Feb 2007 17:35:16 +1100
NeilBrown [EMAIL PROTECTED] wrote:
+for (i = conf-raid_disks ; i-- ; ) {
That statement should be dragged out, shot, stomped on then ceremonially
incinerated.
What's wrong with doing
for (i = 0; i conf-raid_disks; i++) {
in a manner which can be understood without alcoholic fortification?
ho hum.
In case someone likes to do job, GCC usually ought to do, i would
suggest something like this instead:
if (expanded test_bit(STRIPE_EXPANDING, sh-state)) {
/* Need to write out all blocks after computing PQ */
- sh-disks = conf-raid_disks;
+ i = conf-raid_disks;
+ sh-disks = i;
- sh-pd_idx = stripe_to_pdidx(sh-sector, conf,
-conf-raid_disks);
+ sh-pd_idx = stripe_to_pdidx(sh-sector, conf, i);
compute_parity6(sh, RECONSTRUCT_WRITE);
- for (i = conf-raid_disks ; i-- ; ) {
+ do {
set_bit(R5_LOCKED, sh-dev[i].flags);
locked++;
set_bit(R5_Wantwrite, sh-dev[i].flags);
- }
+ } while (--i);
clear_bit(STRIPE_EXPANDING, sh-state);
} else if (expanded) {
In any case this is subject of scripts/bloat-o-meter.
-
To unsubscribe from this list: send the line unsubscribe linux-raid in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
On Thu, 22 Feb 2007 00:36:22 +0100
Oleg Verych [EMAIL PROTECTED] wrote:
From: Andrew Morton
Newsgroups: gmane.linux.raid,gmane.linux.kernel
Subject: Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
Date: Wed, 21 Feb 2007 14:48:06 -0800
Hallo.
On Tue, 20 Feb 2007 17:35:16 +1100
NeilBrown [EMAIL PROTECTED] wrote:
+ for (i = conf-raid_disks ; i-- ; ) {
That statement should be dragged out, shot, stomped on then ceremonially
incinerated.
What's wrong with doing
for (i = 0; i conf-raid_disks; i++) {
in a manner which can be understood without alcoholic fortification?
ho hum.
In case someone likes to do job, GCC usually ought to do, i would
suggest something like this instead:
if (expanded test_bit(STRIPE_EXPANDING, sh-state)) {
/* Need to write out all blocks after computing PQ */
- sh-disks = conf-raid_disks;
+ i = conf-raid_disks;
+ sh-disks = i;
- sh-pd_idx = stripe_to_pdidx(sh-sector, conf,
-conf-raid_disks);
+ sh-pd_idx = stripe_to_pdidx(sh-sector, conf, i);
compute_parity6(sh, RECONSTRUCT_WRITE);
- for (i = conf-raid_disks ; i-- ; ) {
+ do {
set_bit(R5_LOCKED, sh-dev[i].flags);
locked++;
set_bit(R5_Wantwrite, sh-dev[i].flags);
- }
+ } while (--i);
clear_bit(STRIPE_EXPANDING, sh-state);
} else if (expanded) {
In any case this is subject of scripts/bloat-o-meter.
This:
--- a/drivers/md/raid5.c~a
+++ a/drivers/md/raid5.c
@@ -2364,7 +2364,7 @@ static void handle_stripe6(struct stripe
sh-pd_idx = stripe_to_pdidx(sh-sector, conf,
conf-raid_disks);
compute_parity6(sh, RECONSTRUCT_WRITE);
- for (i = conf-raid_disks ; i-- ; ) {
+ for (i = 0; i conf-raid_disks; ++) {
set_bit(R5_LOCKED, sh-dev[i].flags);
locked++;
set_bit(R5_Wantwrite, sh-dev[i].flags);
_
reduces the size of drivers/md/raid5.o's .text by two bytes.
-
To unsubscribe from this list: send the line unsubscribe linux-raid in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
On Thursday, 22 February 2007 00:58, Andrew Morton wrote:
On Thu, 22 Feb 2007 00:36:22 +0100
Oleg Verych [EMAIL PROTECTED] wrote:
From: Andrew Morton
Newsgroups: gmane.linux.raid,gmane.linux.kernel
Subject: Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
Date: Wed, 21 Feb 2007 14:48:06 -0800
Hallo.
On Tue, 20 Feb 2007 17:35:16 +1100
NeilBrown [EMAIL PROTECTED] wrote:
+for (i = conf-raid_disks ; i-- ; ) {
That statement should be dragged out, shot, stomped on then ceremonially
incinerated.
What's wrong with doing
for (i = 0; i conf-raid_disks; i++) {
in a manner which can be understood without alcoholic fortification?
ho hum.
In case someone likes to do job, GCC usually ought to do, i would
suggest something like this instead:
if (expanded test_bit(STRIPE_EXPANDING, sh-state)) {
/* Need to write out all blocks after computing PQ */
- sh-disks = conf-raid_disks;
+ i = conf-raid_disks;
+ sh-disks = i;
- sh-pd_idx = stripe_to_pdidx(sh-sector, conf,
-conf-raid_disks);
+ sh-pd_idx = stripe_to_pdidx(sh-sector, conf, i);
compute_parity6(sh, RECONSTRUCT_WRITE);
- for (i = conf-raid_disks ; i-- ; ) {
+ do {
set_bit(R5_LOCKED, sh-dev[i].flags);
locked++;
set_bit(R5_Wantwrite, sh-dev[i].flags);
- }
+ } while (--i);
clear_bit(STRIPE_EXPANDING, sh-state);
} else if (expanded) {
In any case this is subject of scripts/bloat-o-meter.
This:
--- a/drivers/md/raid5.c~a
+++ a/drivers/md/raid5.c
@@ -2364,7 +2364,7 @@ static void handle_stripe6(struct stripe
sh-pd_idx = stripe_to_pdidx(sh-sector, conf,
conf-raid_disks);
compute_parity6(sh, RECONSTRUCT_WRITE);
- for (i = conf-raid_disks ; i-- ; ) {
+ for (i = 0; i conf-raid_disks; ++) {
I guess it should be
+ for (i = 0; i conf-raid_disks; i++)
set_bit(R5_LOCKED, sh-dev[i].flags);
locked++;
set_bit(R5_Wantwrite, sh-dev[i].flags);
_
-
To unsubscribe from this list: send the line unsubscribe linux-raid in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
On Wednesday February 21, [EMAIL PROTECTED] wrote:
On Tue, 20 Feb 2007 17:35:16 +1100
NeilBrown [EMAIL PROTECTED] wrote:
+ for (i = conf-raid_disks ; i-- ; ) {
That statement should be dragged out, shot, stomped on then ceremonially
incinerated.
An experiment in lateral thinking? I liked it, but there is no
accounting for taste.
What's wrong with doing
for (i = 0; i conf-raid_disks; i++) {
in a manner which can be understood without alcoholic fortification?
I guess... Egoless programmer and all that, write for others to
read, not for the compiler, and as you say it comes to the same
number of bytes of code on common architectures.
ho hum.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
I must right code that Andrew can read.
NeilBrown
-
To unsubscribe from this list: send the line unsubscribe linux-raid in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 006 of 6] md: Add support for reshape of a raid6
On Thu, 22 Feb 2007 13:39:56 +1100 Neil Brown [EMAIL PROTECTED] wrote: I must right code that Andrew can read. That's write. But more importantly, things that people can immediately see and understand help reduce the possibility of mistakes. Now and in the future. If we did all loops like that, then it'd be the the best way to do it in new code, because people's eyes and brains are locked into that idiom and we just don't have to think about it when we see it. - To unsubscribe from this list: send the line unsubscribe linux-raid in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 006 of 6] md: Add support for reshape of a raid6
i.e. one or more drives can be added and the array will re-stripe
while on-line.
Most of the interesting work was already done for raid5.
This just extends it to raid6.
mdadm newer than 2.6 is needed for complete safety, however any
version of mdadm which support raid5 reshape will do a good enough job
in almost all cases (an 'echo repair /sys/block/mdX/md/sync_action'
is recommended after a reshape that was aborted and had to be
restarted with an such a version of mdadm).
Signed-off-by: Neil Brown [EMAIL PROTECTED]
### Diffstat output
./drivers/md/raid5.c | 157 ---
1 file changed, 124 insertions(+), 33 deletions(-)
diff .prev/drivers/md/raid5.c ./drivers/md/raid5.c
--- .prev/drivers/md/raid5.c2007-02-20 17:14:35.0 +1100
+++ ./drivers/md/raid5.c2007-02-20 17:14:48.0 +1100
@@ -1050,7 +1050,7 @@ static void compute_parity5(struct strip
static void compute_parity6(struct stripe_head *sh, int method)
{
raid6_conf_t *conf = sh-raid_conf;
- int i, pd_idx = sh-pd_idx, qd_idx, d0_idx, disks = conf-raid_disks,
count;
+ int i, pd_idx = sh-pd_idx, qd_idx, d0_idx, disks = sh-disks, count;
struct bio *chosen;
/ FIX THIS: This could be very bad if disks is close to 256 /
void *ptrs[disks];
@@ -1131,8 +1131,7 @@ static void compute_parity6(struct strip
/* Compute one missing block */
static void compute_block_1(struct stripe_head *sh, int dd_idx, int nozero)
{
- raid6_conf_t *conf = sh-raid_conf;
- int i, count, disks = conf-raid_disks;
+ int i, count, disks = sh-disks;
void *ptr[MAX_XOR_BLOCKS], *p;
int pd_idx = sh-pd_idx;
int qd_idx = raid6_next_disk(pd_idx, disks);
@@ -1170,8 +1169,7 @@ static void compute_block_1(struct strip
/* Compute two missing blocks */
static void compute_block_2(struct stripe_head *sh, int dd_idx1, int dd_idx2)
{
- raid6_conf_t *conf = sh-raid_conf;
- int i, count, disks = conf-raid_disks;
+ int i, count, disks = sh-disks;
int pd_idx = sh-pd_idx;
int qd_idx = raid6_next_disk(pd_idx, disks);
int d0_idx = raid6_next_disk(qd_idx, disks);
@@ -1887,11 +1885,11 @@ static void handle_stripe5(struct stripe
static void handle_stripe6(struct stripe_head *sh, struct page *tmp_page)
{
raid6_conf_t *conf = sh-raid_conf;
- int disks = conf-raid_disks;
+ int disks = sh-disks;
struct bio *return_bi= NULL;
struct bio *bi;
int i;
- int syncing;
+ int syncing, expanding, expanded;
int locked=0, uptodate=0, to_read=0, to_write=0, failed=0, written=0;
int non_overwrite = 0;
int failed_num[2] = {0, 0};
@@ -1909,6 +1907,8 @@ static void handle_stripe6(struct stripe
clear_bit(STRIPE_DELAYED, sh-state);
syncing = test_bit(STRIPE_SYNCING, sh-state);
+ expanding = test_bit(STRIPE_EXPAND_SOURCE, sh-state);
+ expanded = test_bit(STRIPE_EXPAND_READY, sh-state);
/* Now to look around and see what can be done */
rcu_read_lock();
@@ -2114,13 +2114,15 @@ static void handle_stripe6(struct stripe
* parity, or to satisfy requests
* or to load a block that is being partially written.
*/
- if (to_read || non_overwrite || (to_write failed) || (syncing
(uptodate disks))) {
+ if (to_read || non_overwrite || (to_write failed) ||
+ (syncing (uptodate disks)) || expanding) {
for (i=disks; i--;) {
dev = sh-dev[i];
if (!test_bit(R5_LOCKED, dev-flags)
!test_bit(R5_UPTODATE, dev-flags)
(dev-toread ||
(dev-towrite !test_bit(R5_OVERWRITE,
dev-flags)) ||
syncing ||
+expanding ||
(failed = 1 (sh-dev[failed_num[0]].toread ||
to_write)) ||
(failed = 2 (sh-dev[failed_num[1]].toread ||
to_write))
)
@@ -2355,6 +2357,79 @@ static void handle_stripe6(struct stripe
}
}
}
+
+ if (expanded test_bit(STRIPE_EXPANDING, sh-state)) {
+ /* Need to write out all blocks after computing PQ */
+ sh-disks = conf-raid_disks;
+ sh-pd_idx = stripe_to_pdidx(sh-sector, conf,
+conf-raid_disks);
+ compute_parity6(sh, RECONSTRUCT_WRITE);
+ for (i = conf-raid_disks ; i-- ; ) {
+ set_bit(R5_LOCKED, sh-dev[i].flags);
+ locked++;
+ set_bit(R5_Wantwrite, sh-dev[i].flags);
+ }
+ clear_bit(STRIPE_EXPANDING, sh-state);
+ } else if (expanded) {
+ clear_bit(STRIPE_EXPAND_READY, sh-state);
+
