Skip to contentSkip to conversation

raid1: set bi_idx after bio cloning

PATCH In review Mike Tran 4 participants 5 messages May 13, 2005 – May 23, 2005

Subsystems (2)
lore
Share & tools
lore

Mike Tran

author
#0 lore raw

PATCH 1/1 raid1: set bi_idx after bio cloning

Hi Neil et al,

The scenario:  Having LVM (striped) volumes on top of 2 raid1 devices
The problem:   Corruption in building the scatter gather list for

device driver (see /drivers/block/blk_rq_map_sg() function)

In DM code, a bio sometimes can be split into several clones and the clones' bi_idx is set accordingly by DM. MD raid1 also clones bio but does not adjust bi_idx. The problem can be fixed in either __bio_clone() or the caller of bio_clone(). I chose to fix raid1.

drivers/md/raid1.c 2005-05-13 15:41:43.000000000-0500 renamed from drivers/md/raid1.c 2005-05-10 13:35:06.000000000-0500 +1 -0
@@ -577,6 +577,7 @@
 
                r1_bio->bios[rdisk] = read_bio;
 
+               read_bio->bi_idx = bio->bi_idx;
                read_bio->bi_sector = r1_bio->sector + mirror->rdev->data_offset;
                read_bio->bi_bdev = mirror->rdev->bdev;
                read_bio->bi_end_io = raid1_end_read_request;

--
Regards,
Mike T.

#1 lore raw
Quoted 11 lines · On Friday May 13, [email protected] wrote:

Hi Neil et al,

The scenario:  Having LVM (striped) volumes on top of 2 raid1 devices
The problem:   Corruption in building the scatter gather list for

device driver (see /drivers/block/blk_rq_map_sg() function)

In DM code, a bio sometimes can be split into several clones and the clones' bi_idx is set accordingly by DM. MD raid1 also clones bio but does not adjust bi_idx. The problem can be fixed in either __bio_clone() or the caller of bio_clone(). I chose to fix raid1.

Presumably the same fix would be needed in the "write" path and all in the read-retry path in raid1d?

Why not just put it in __bio_clone, which would be one change instead of (atleast) 3?

NeilBrown

Quoted 16 lines
drivers/md/raid1.c 2005-05-13 15:41:43.000000000-0500 renamed from drivers/md/raid1.c 2005-05-10 13:35:06.000000000-0500 +1 -1
@@ -577,6 +577,7 @@
 
                r1_bio->bios[rdisk] = read_bio;
 
+               read_bio->bi_idx = bio->bi_idx;
                read_bio->bi_sector = r1_bio->sector + mirror->rdev->data_offset;
                read_bio->bi_bdev = mirror->rdev->bdev;
                read_bio->bi_end_io = raid1_end_read_request;
 
--

Regards,
Mike T.

author
#2 lore raw

Re: [PATCH] bio: set bi_idx after bio cloning

Hi Jens,

Unless you have specific reason for not doing in __bio_clone(), I agree with Neil's suggestion. Please consider the following patch:

fs/bio.c 2005-05-16 10:15:06.000000000 -0500 renamed from fs/bio.c 2005-05-16 10:13:27.000000000 -0500 +1 -4
@@ -255,10 +255,7 @@
        bio->bi_flags |= 1 << BIO_CLONED;
        bio->bi_rw = bio_src->bi_rw;
 
-       /*
-        * notes -- maybe just leave bi_idx alone. assume identical mapping
-        * for the clone
-        */
+       bio->bi_idx = bio_src->bi_idx;
        bio->bi_vcnt = bio_src->bi_vcnt;
        bio->bi_size = bio_src->bi_size;
        bio_phys_segments(q, bio);

--
Regards,
Mike T.

Quoted 42 lines · Neil Brown
Quoted 11 lines · On Friday May 13, [email protected] wrote:

Hi Neil et al,

The scenario:  Having LVM (striped) volumes on top of 2 raid1 devices
The problem:   Corruption in building the scatter gather list for

device driver (see /drivers/block/blk_rq_map_sg() function)

In DM code, a bio sometimes can be split into several clones and the clones' bi_idx is set accordingly by DM. MD raid1 also clones bio but does not adjust bi_idx. The problem can be fixed in either __bio_clone() or the caller of bio_clone(). I chose to fix raid1.

Presumably the same fix would be needed in the "write" path and all in the read-retry path in raid1d?

Why not just put it in __bio_clone, which would be one change instead of (atleast) 3?

NeilBrown

Quoted 13 lines
drivers/md/raid1.c 2005-05-13 15:41:43.000000000-0500 renamed from drivers/md/raid1.c 2005-05-10 13:35:06.000000000-0500 +1 -1
@@ -577,6 +577,7 @@
 
                r1_bio->bios[rdisk] = read_bio;
 
+               read_bio->bi_idx = bio->bi_idx;
                read_bio->bi_sector = r1_bio->sector + mirror->rdev->data_offset;
                read_bio->bi_bdev = mirror->rdev->bdev;
                read_bio->bi_end_io = raid1_end_read_request;
 
--
Quoted 3 lines

Regards,
Mike T.

-
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

#3 lore raw

problems with 3ware 8506-8 post-disk failure

Scenario:
dual opteron/4G/Ubuntu pure 64bit SMP / OS on separate IDE drive, 3ware 8506-8port driving 8x WD2500JD disks in Chenbro hotswap cages as RAID5, config'ed as both reiserfs (pre-catastrophe) and ext3 (postcatastrophe). I'm responsible for getting this system up (done) and reliable (not done).

The short version is that it ran well for a few weeks until we discovered on a reboot that a disk had silently failed, degrading the RAID5. In trying to repair that failure, 3ware's 3dm2 software that indicated that it was repairing the array, but failed to do so, causing the loss of the entire
array. I tried to rescue the data with reiserfs's fsck but was only able to
recover individual chunks. Since most of the info was huge binary files and most of it was backed up elsewhere, we decided not to attempt to rescue anything and we re-formatted with ext3, supposedly bc it was considered more reliable and better suited for large files. After that, the raid stayed up for a day or so and I loaded it down with huge disk i/o, trying to see what would happen. The same port / disk # failed again (tho at least this time the SW notified us), but this seems pretty suspicious that it's the same port number failing.

I played around with the motherboard Silicon Image 4port SATA controller and sw raid (via mdadm) for a while and found that after a certain amount of futzing, it looked not too bad, but the amount of futzing made me a bit nervous, especially since someone else is going to have to care for it. The speed of the SW RAID was about 10-20% better than the 3ware by bonnie++, but I liked the idea on having the RAID looks like big scsi disk. So I went for the 3ware.

I'll detail the complete catastrophe later (already written up in large chunks
- just have to remove some inflammatory language before posting), but my question to the group is what people think of 3ware's support. The common opinion on 3ware seems to be that it's great that they support Linux and the HW works fine (also my experience), but my opinion has been shaded considerably by what happens when a RAID fails - when you really DO need to recover and you need a straightforward path to do so.

In short, I've found 3ware support for recovery procedures to be hard to find (via google for example and also on their website), hard to understand because of some peculiar nomenclature, and sometimes misleading due to oddities of their software.

Is this just my experience, or is this a widely held view? I realize that I'm talking to a group that seems to be heavily weighted towards SW RAID, but maybe it's just me. If anyone can compare recovery paths between the 2 (SW vs 3ware HW) I'd be very happy to hear the stories. Given this recent experience, I'm re-evaluating whether I should switch back and go SW RAID, especially given another large catastrophe involving 3ware ccontrollers on campus.

Have people found that the Chenbro hotswap cages are a contributing factor to RAID failure? That's what one 3wware person indicated.

Signature · 3 lines
Cheers, Harry
Harry J Mangalam - 949 856 2847 (vox; email for fax) - [email protected] 
            <<plain text preferred>>
#4 lore raw

Re: [PATCH] bio: set bi_idx after bio cloning

In reply to Mike Tran
Quoted 10 lines · Mike Tran

Hi Jens,

Unless you have specific reason for not doing in __bio_clone(), I agree with Neil's suggestion. Please consider the following patch:

fs/bio.c 2005-05-16 10:15:06.000000000 -0500 renamed from fs/bio.c 2005-05-16 10:13:27.000000000 -0500 +0 -0
@@ -255,10 +255,7 @@
        bio->bi_flags |= 1 << BIO_CLONED;

fs/bio.c 2005-05-16 10:15:06.000000000 -0500 line 261

Quoted 10 lines
fs/bio.c 2005-05-16 10:15:06.000000000 -0500 renamed from fs/bio.c 2005-05-16 10:13:27.000000000 -0500 +1 -4
        bio->bi_rw = bio_src->bi_rw;
 
-       /*
-        * notes -- maybe just leave bi_idx alone. assume identical mapping
-        * for the clone
-        */
+       bio->bi_idx = bio_src->bi_idx;
        bio->bi_vcnt = bio_src->bi_vcnt;
        bio->bi_size = bio_src->bi_size;
        bio_phys_segments(q, bio);

That patch is fine with me, I'll add it. Thanks.

Signature · 1 line
Jens Axboe