raid5: When add stripe_head to inactive_list, it should remove hash.
If it released stripe_head to inactive_list and soon get it from hash list. So the stripe_head didn't call init_stripe, the dev.flags isn't clear. So the debug info is error which printed the before info. Although, i can't find logic error because this.I think it should be correct.
--- drivers/md/raid5.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
drivers/md/raid5.c +19 -16
@@ -196,6 +196,24 @@ static int stripe_operations_active(struct stripe_head *sh)test_bit (STRIPE_COMPUTE_RUN , &sh->state); } +static inline void remove_hash (struct stripe_head *sh) +{ +pr_debug ("remove_hash(), stripe %llu\n" , + (unsigned long long )sh->sector); + +hlist_del_init (&sh->hash); +} + +static inline void insert_hash (struct r5conf *conf,struct stripe_head *sh) +{ +struct hlist_head *hp =stripe_hash (conf, sh->sector); + +pr_debug ("insert_hash(), stripe %llu\n" , + (unsigned long long )sh->sector); + +hlist_add_head (&sh->hash, hp); +} +static void do_release_stripe (struct r5conf *conf,struct stripe_head *sh) {BUG_ON (!list_empty (&sh->lru));
@@ -222,6 +240,7 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh)atomic_dec (&conf->active_stripes);if (!test_bit (STRIPE_EXPANDING , &sh->state)) {list_add_tail (&sh->lru, &conf->inactive_list); +remove_hash (sh);wake_up (&conf->wait_for_stripe);if (conf->retry_read_aligned)md_wakeup_thread (conf->mddev->thread);
@@ -248,23 +267,7 @@ static void release_stripe(struct stripe_head *sh)local_irq_restore (flags); } -static inline void remove_hash (struct stripe_head *sh) -{ -pr_debug ("remove_hash(), stripe %llu\n" , - (unsigned long long )sh->sector); -hlist_del_init (&sh->hash); -} - -static inline void insert_hash (struct r5conf *conf,struct stripe_head *sh) -{ -struct hlist_head *hp =stripe_hash (conf, sh->sector); - -pr_debug ("insert_hash(), stripe %llu\n" , - (unsigned long long )sh->sector); - -hlist_add_head (&sh->hash, hp); -}/* find an idle stripe, make sure it is unhashed, and return it. */
Signature · 2 lines
1.7.9.5
On 2012-08-18 10:53 Jianpeng Ma <[email protected]> Wrote:
Quoted 75 lines
If it released
stripe_headtoinactive_listand soon get it from hash list. So thestripe_headdidn't callinit_stripe, the dev.flags isn't clear. So the debug info is error which printed the before info. Although, i can't find logic error because this.I think it should be correct.Signed-off-by Jianpeng Ma <[email protected]>---
drivers/md/raid5.c| 35 +++++++++++++++++++----------------1 file changed, 19 insertions(+), 16 deletions(-)drivers/md/raid5.c +19
-16@@ -196,6 +196,24 @@ static int stripe_operations_active(struct stripe_head *sh)test_bit (STRIPE_COMPUTE_RUN , &sh->state); } +static inline void remove_hash (struct stripe_head *sh) +{ +pr_debug ("remove_hash(), stripe %llu\n" , + (unsigned long long )sh->sector); + +hlist_del_init (&sh->hash); +} + +static inline void insert_hash (struct r5conf *conf,struct stripe_head *sh) +{ +struct hlist_head *hp =stripe_hash (conf, sh->sector); + +pr_debug ("insert_hash(), stripe %llu\n" , + (unsigned long long )sh->sector); + +hlist_add_head (&sh->hash, hp); +} +static void do_release_stripe (struct r5conf *conf,struct stripe_head *sh) {BUG_ON (!list_empty (&sh->lru));@@ -222,6 +240,7 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh)atomic_dec (&conf->active_stripes);if (!test_bit (STRIPE_EXPANDING , &sh->state)) {list_add_tail (&sh->lru, &conf->inactive_list); +remove_hash (sh);wake_up (&conf->wait_for_stripe);if (conf->retry_read_aligned)md_wakeup_thread (conf->mddev->thread);@@ -248,23 +267,7 @@ static void release_stripe(struct stripe_head *sh)local_irq_restore (flags); } -static inline void remove_hash (struct stripe_head *sh) -{ -pr_debug ("remove_hash(), stripe %llu\n" , - (unsigned long long )sh->sector); -hlist_del_init (&sh->hash); -} - -static inline void insert_hash (struct r5conf *conf,struct stripe_head *sh) -{ -struct hlist_head *hp =stripe_hash (conf, sh->sector); - -pr_debug ("insert_hash(), stripe %llu\n" , - (unsigned long long )sh->sector); - -hlist_add_head (&sh->hash, hp); -}/* find an idle stripe, make sure it is unhashed, and return it. */ Signature · 1 line
1.7.9.5
Hi neil:
Today, i found a problem about this.
1:mdadm -CR /dev/md0 -l5 -c4 -n4 /dev/sd[bcd] missing
2:dd if=/dev/zero of=/dev/md0 bs=4K count=1 oflag=direct 3:hdparm --make-bad-sector 2048 --yes-i-know-what-i-am-doing /dev/sdb
(2048 is the data-offset of sdb)
4:dd if=/dev/md0 of=/dev/zero bs=4K count=1 iflag=direct
I think step4 maybe return ioerror.But it's success. The kernel message like :
[19277.581245] ata3.00: exception Emask 0x0 SAct 0x1 SErr 0x0 action 0x0
[19277.581250] ata3.00: irq_stat 0x40000008
[19277.581253] ata3.00: failed command: READ FPDMA QUEUED
[19277.581260] ata3.00: cmd 60/08:00:00:08:00/00:00:00:00:00/40 tag 0 ncq 4096 in
[19277.581260] res 41/40:08:00:08:00/00:00:00:00:00/00 Emask 0x409 (media error) <F>
[19277.581263] ata3.00: status: { DRDY ERR }
[19277.581265] ata3.00: error: { UNC }
[19277.585809] ata3.00: configured for UDMA/133
[19277.585828] sd 2:0:0:0: \x010[sdb] Unhandled sense code
[19277.585831] sd 2:0:0:0: \x010[sdb]
[19277.585833] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
[19277.585835] sd 2:0:0:0: \x010[sdb]
[19277.585837] Sense Key : Medium Error [current] [descriptor]
[19277.585842] Descriptor sense data with sense descriptors (in hex):
[19277.585844] 72 03 11 04 00 00 00 0c 00 0a 80 00 00 00 00 00
[19277.585856] 00 00 08 00
[19277.585862] sd 2:0:0:0: \x010[sdb]
[19277.585864] Add. Sense: Unrecovered read error - auto reallocate failed
[19277.585867] sd 2:0:0:0: \x010[sdb] CDB:
[19277.585869] Read(10): 28 00 00 00 08 00 00 00 08 00
[19277.585879] end_request: I/O error, dev sdb, sector 2048
[19277.585889] raid5_align_endio : io error...handing IO for a retry
[19277.585901] +++ raid5d active
[19277.585906] get_stripe, sector 0
[19277.585908] __find_stripe, sector 0
[19277.585911] adding bi b#0 to stripe s#0
[19277.585914] added bi b#0 to stripe s#0, disk 0.
[19277.585918] handling stripe 0, state=0x1091 cnt=1, pd_idx=3, qd_idx=-1
[19277.585918] , check:0, reconstruct:0
[19277.585922] check 3: state 0x1 read (null) write (null) written (null)
[19277.585925] check 2: state 0x11 read (null) write (null) written (null)
[19277.585927] ata3: EH complete
[19277.585930] check 1: state 0x11 read (null) write (null) written (null)
[19277.585933] check 0: state 0x119 read ffff8800b69cd900 write (null) written (null)
[19277.585936] locked=0 uptodate=4 to_read=0 to_write=0 failed=1 failed_num=3,-1
[19277.585939] ops_run_biofill: stripe 0
[19277.585944] ops_complete_biofill: stripe 0
[19277.585955] __get_priority_stripe: handle: busy hold: empty full_writes: 0 bypass_count: 0
[19277.585958] __get_priority_stripe: handle: empty hold: empty full_writes: 0 bypass_count: 0
[19277.585961] handling stripe 0, state=0x1091 cnt=1, pd_idx=3, qd_idx=-1
[19277.585961] , check:0, reconstruct:0
[19277.585964] check 3: state 0x1 read (null) write (null) written (null)
[19277.585967] check 2: state 0x11 read (null) write (null) written (null)
[19277.585969] check 1: state 0x11 read (null) write (null) written (null)
[19277.585972] check 0: state 0x119 read (null) write (null) written (null)
[19277.585975] locked=0 uptodate=4 to_read=0 to_write=0 failed=1 failed_num=3,-1
[19277.585979] __get_priority_stripe: handle: empty hold: empty full_writes: 0 bypass_count: 0
[19277.585981] 2 stripes handled
[19277.585983] --- raid5d inactive
[19277.585984] +++ raid5d active
[19277.585987] __get_priority_stripe: handle: empty hold: empty full_writes: 0 bypass_count: 0
[19277.585989] 0 stripes handled
[19277.585991] --- raid5d inactiveQuoted 84 lines · Jianpeng Ma
On 2012-08-18 10:53 Jianpeng Ma <[email protected]> Wrote:
Quoted 73 lines
If it released
stripe_headtoinactive_listand soon get it from hash list. So thestripe_headdidn't callinit_stripe, the dev.flags isn't clear. So the debug info is error which printed the before info. Although, i can't find logic error because this.I think it should be correct.Signed-off-by Jianpeng Ma <[email protected]>---
drivers/md/raid5.c| 35 +++++++++++++++++++----------------1 file changed, 19 insertions(+), 16 deletions(-)drivers/md/raid5.c +19
-17@@ -196,6 +196,24 @@ static int stripe_operations_active(struct stripe_head *sh)test_bit (STRIPE_COMPUTE_RUN , &sh->state); } +static inline void remove_hash (struct stripe_head *sh) +{ +pr_debug ("remove_hash(), stripe %llu\n" , + (unsigned long long )sh->sector); + +hlist_del_init (&sh->hash); +} + +static inline void insert_hash (struct r5conf *conf,struct stripe_head *sh) +{ +struct hlist_head *hp =stripe_hash (conf, sh->sector); + +pr_debug ("insert_hash(), stripe %llu\n" , + (unsigned long long )sh->sector); + +hlist_add_head (&sh->hash, hp); +} +static void do_release_stripe (struct r5conf *conf,struct stripe_head *sh) {BUG_ON (!list_empty (&sh->lru));@@ -222,6 +240,7 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh)atomic_dec (&conf->active_stripes);if (!test_bit (STRIPE_EXPANDING , &sh->state)) {list_add_tail (&sh->lru, &conf->inactive_list); +remove_hash (sh);wake_up (&conf->wait_for_stripe);if (conf->retry_read_aligned)md_wakeup_thread (conf->mddev->thread);@@ -248,23 +267,7 @@ static void release_stripe(struct stripe_head *sh)local_irq_restore (flags); } -static inline void remove_hash (struct stripe_head *sh) -{ -pr_debug ("remove_hash(), stripe %llu\n" , - (unsigned long long )sh->sector); -hlist_del_init (&sh->hash); -} - -static inline void insert_hash (struct r5conf *conf,struct stripe_head *sh) -{ -struct hlist_head *hp =stripe_hash (conf, sh->sector); - -pr_debug ("insert_hash(), stripe %llu\n" , - (unsigned long long )sh->sector); - -hlist_add_head (&sh->hash, hp); -}/* find an idle stripe, make sure it is unhashed, and return it. */ --Quoted 2 lines
1.7.9.5
Hi neil:
Today, i found a problem about this.
1:mdadm -CR/dev/md0-l5 -c4 -n4 /dev/sd[bcd] missing
2:dd if=/dev/zero of=/dev/md0 bs=4K count=1 oflag=direct 3:hdparm --make-bad-sector 2048 --yes-i-know-what-i-am-doing/dev/sdb
(2048 is the data-offset of sdb)
4:dd if=/dev/md0 of=/dev/zero bs=4K count=1 iflag=direct
Quoted 1 line
I think step4 maybe return ioerror.But it's success.
You would need to check that stripe0 got recycled between step2 and step3. If not then the uptodate stripe is in the cache and the read can be serviced from the cached data.
...which seems to be the case because of:
Quoted 1 line
[19277.585936] locked=0 uptodate=4 to_read=0 to_write=0 failed=1 failed_num=3,-1
--
Dan
On 2012-08-21 01:51 Dan Williams <[email protected]> Wrote:
Quoted 91 lines · Jianpeng Ma
Quoted 84 lines
On 2012-08-18 10:53 Jianpeng Ma <[email protected]> Wrote:
Quoted 73 lines
If it released
stripe_headtoinactive_listand soon get it from hash list. So thestripe_headdidn't callinit_stripe, the dev.flags isn't clear. So the debug info is error which printed the before info. Although, i can't find logic error because this.I think it should be correct.Signed-off-by Jianpeng Ma <[email protected]>---
drivers/md/raid5.c| 35 +++++++++++++++++++----------------1 file changed, 19 insertions(+), 16 deletions(-)drivers/md/raid5.c +19
-17@@ -196,6 +196,24 @@ static int stripe_operations_active(struct stripe_head *sh)test_bit (STRIPE_COMPUTE_RUN , &sh->state); } +static inline void remove_hash (struct stripe_head *sh) +{ +pr_debug ("remove_hash(), stripe %llu\n" , + (unsigned long long )sh->sector); + +hlist_del_init (&sh->hash); +} + +static inline void insert_hash (struct r5conf *conf,struct stripe_head *sh) +{ +struct hlist_head *hp =stripe_hash (conf, sh->sector); + +pr_debug ("insert_hash(), stripe %llu\n" , + (unsigned long long )sh->sector); + +hlist_add_head (&sh->hash, hp); +} +static void do_release_stripe (struct r5conf *conf,struct stripe_head *sh) {BUG_ON (!list_empty (&sh->lru));@@ -222,6 +240,7 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh)atomic_dec (&conf->active_stripes);if (!test_bit (STRIPE_EXPANDING , &sh->state)) {list_add_tail (&sh->lru, &conf->inactive_list); +remove_hash (sh);wake_up (&conf->wait_for_stripe);if (conf->retry_read_aligned)md_wakeup_thread (conf->mddev->thread);@@ -248,23 +267,7 @@ static void release_stripe(struct stripe_head *sh)local_irq_restore (flags); } -static inline void remove_hash (struct stripe_head *sh) -{ -pr_debug ("remove_hash(), stripe %llu\n" , - (unsigned long long )sh->sector); -hlist_del_init (&sh->hash); -} - -static inline void insert_hash (struct r5conf *conf,struct stripe_head *sh) -{ -struct hlist_head *hp =stripe_hash (conf, sh->sector); - -pr_debug ("insert_hash(), stripe %llu\n" , - (unsigned long long )sh->sector); - -hlist_add_head (&sh->hash, hp); -}/* find an idle stripe, make sure it is unhashed, and return it. */ --Quoted 2 lines
1.7.9.5
Hi neil:
Today, i found a problem about this.
1:mdadm -CR/dev/md0-l5 -c4 -n4 /dev/sd[bcd] missing
2:dd if=/dev/zero of=/dev/md0 bs=4K count=1 oflag=direct 3:hdparm --make-bad-sector 2048 --yes-i-know-what-i-am-doing/dev/sdb
(2048 is the data-offset of sdb)
4:dd if=/dev/md0 of=/dev/zero bs=4K count=1 iflag=directQuoted 1 line
I think step4 maybe return ioerror.But it's success.
You would need to check that stripe0 got recycled between step2 and step3. If not then the uptodate stripe is in the cache and the read can be serviced from the cached data.
Quoted 6 lines
...which seems to be the case because of:
Quoted 1 line
[19277.585936] locked=0 uptodate=4 to_read=0 to_write=0 failed=1 failed_num=3,-1--
Dan
Thanks! If stripe had a cache function.I think the chunk_aligned_read should use it for small read situation.
Quoted 8 lines · Jianpeng Ma
On 2012-08-21 01:51 Dan Williams <[email protected]> Wrote:
Quoted 7 lines · Jianpeng Ma
Quoted 1 line
I think step4 maybe return ioerror.But it's success.
You would need to check that stripe0 got recycled between step2 and step3. If not then the uptodate stripe is in the cache and the read can be serviced from the cached data.
Quoted 6 lines
Quoted 4 lines
...which seems to be the case because of:
Quoted 1 line
[19277.585936] locked=0 uptodate=4 to_read=0 to_write=0 failed=1 failed_num=3,-1Thanks! If stripe had a cache function.I think the
chunk_aligned_readshould use it for small read situation.
But the stripe cache is not meant to be a read cache as it would be duplicating what the page cache offers. Most aligned reads are likely to miss the stripe cache so adding a lookup for every read would be mostly wasted overhead.
--
Dan
On 2012-08-22 00:43 Dan Williams <[email protected]> Wrote:
Quoted 14 lines · Jianpeng Ma
Quoted 13 lines
On 2012-08-21 01:51 Dan Williams <[email protected]> Wrote:
Quoted 11 lines · Jianpeng Ma
Quoted 1 line
I think step4 maybe return ioerror.But it's success.
You would need to check that stripe0 got recycled between step2 and step3. If not then the uptodate stripe is in the cache and the read can be serviced from the cached data.
...which seems to be the case because of:
Quoted 1 line
[19277.585936] locked=0 uptodate=4 to_read=0 to_write=0 failed=1 failed_num=3,-1
Quoted 7 lines
Quoted 1 line
Thanks! If stripe had a cache function.I think the
chunk_aligned_readshould use it for small read situation.But the stripe cache is not meant to be a read cache as it would be duplicating what the page cache offers. Most aligned reads are likely to miss the stripe cache so adding a lookup for every read would be mostly wasted overhead.
I think most aligned reads are likely to missed the stripe cache because they did not use stripe-cache(when fristly reaed) For direct-io workload, there is no page-cache.
FYI, most hardware raid-controlers have own memory and don't use host-memeory. The stripe-cache of raid456 is the same as raid-controller memeory.They also should provide the page-cache for read, rather only for write.
Quoted 35 lines
-----Original Message-----
From: [email protected] [mailto:linux-raid- [email protected]] On Behalf Of kedacomkernel
Sent: Tuesday, August 21, 2012 6:26 PM
To: Dan Williams; majianpeng
Cc: Neil Brown; linux-raid
Subject: Re: Re: raid5: When addstripe_headtoinactive_list, it should remove hash.On 2012-08-22 00:43 Dan Williams <[email protected]> Wrote:
Quoted 1 line
On Mon, Aug 20, 2012 at 8:14 PM, Jianpeng Ma <[email protected]>
Quoted 2 lines · wrote:
Quoted 2 lines
On 2012-08-21 01:51 Dan Williams <[email protected]> Wrote:
Quoted 1 line
On Mon, Aug 20, 2012 at 1:17 AM, Jianpeng Ma
Quoted 6 lines · <[email protected]> wrote:
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
I think step4 maybe return ioerror.But it's success.
You would need to check that stripe0 got recycled between step2 and step3. If not then the uptodate stripe is in the cache and the read can be serviced from the cached data.
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
...which seems to be the case because of:
Quoted 2 lines
[19277.585936] locked=0 uptodate=4 to_read=0 to_write=0 failed=1failed_num=3,-1
Thanks! If stripe had a cache function.I think the
chunk_aligned_readshould use it for small read situation.
Quoted 6 lines
But the stripe cache is not meant to be a read cache as it would be duplicating what the page cache offers. Most aligned reads are likely to miss the stripe cache so adding a lookup for every read would be mostly wasted overhead.
Quoted 7 lines
I think most aligned reads are likely to missed the stripe cache because they did not use stripe-cache(when fristly reaed) For direct-io workload, there is no page-cache.
FYI, most hardware raid-controlers have own memory and don't use host- memeory. The stripe-cache of raid456 is the same as raid-controller memeory.They also should provide the page-cache for read, rather only for write.
So if a portion of the system memory can be battery backed (something like the ADR feature on the Intel Xeon E5 series), would it be possible for us to put the stripe cache in that battery protected region and make it a protected write back cache to improve performance and provide data protection for software RAID on Linux?