Re: "Robust Read"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Peter T. Breuer wrote:
Michael Tokarev <mjt@xxxxxxxxxx> wrote:

[-- text/plain, encoding 7bit, charset: KOI8-R, 74 lines --]

Uh-oh. Setting proper charset this time. What a nonsense: 7bit but koi8... ;)

[]
Uh OK. As I recall one only needs to count, one doesn't need a bitwise
map of what one has dealt with.

Well. I see read_balance() is now used to resubmit reads. There's a reason to use it instead of choosing "next" disk, I think. There's a reason to NOT use it too, as the "bitmap" will not be needed in that case, only the counter. Both will do the task ofcourse, and the case wich requires only counter is preferred because it does not restrict number of disks in the array the way "bitmap" does.

Ok, so be it the "map" variant when.. and it's even simpler that way.

[big snip]

and then in the raid1d function:

BTW, what's the reason to use raid1d for retries in the first place? Why not just resubmit the request in reschedule_retry() directly, the same way it is done in raid1d?

[]
Yes, I don't think you should muck with read_balance at all! It's an
approach, but I don't think it's the right one. We just want to use the
function to change target disks with here.  Why not reinstate the old
"map" function which can do just that? Then no change to read_balancce
is needed. Rename map() to remap() if it sounds better!

The key point is that there should be not many read errors. Given said that, I agree, simpler routine for retries is ok.

[]
@@ -545,6 +551,7 @@ static int make_request(request_queue_t r1_bio->sector = bio->bi_sector;

       r1_bio->state = 0;
+       r1_bio->tried_disks = 0;

Hmm. Ditto. I suppose we are making the read request here. Yes we are. I had set the "remaining" count instead.

BTW there's no reason to zero any fields in r1_bio because of the way it is allocated (with memset(0) all of it).

[]
What do you think of my comments above?

Ok.

But..  It's very difficult to NOT go into numerous-cleanups mode now.
I want to clean up the code, to make it better first!.. ;)  And I
pretty much sure it will conflict with ongoing work Neil is doing... ;)

/mjt
-
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux