On Tue, Apr 9, 2019 at 2:55 AM jianchao.wang <jianchao.w.wang@xxxxxxxxxx> wrote: > > Hi all > > In analyse_stripe > > is_bad = is_badblock(rdev, sh->sector, STRIPE_SECTORS, > &first_bad, &bad_sectors); > > clear_bit(R5_Insync, &dev->flags); > if (!rdev) > /* Not in-sync */; > else if (is_bad) { > /* also not in-sync */ > if (!test_bit(WriteErrorSeen, &rdev->flags) && > test_bit(R5_UPTODATE, &dev->flags)) { > /* treat as in-sync, but with a read error > * which we can now try to correct > */ > set_bit(R5_Insync, &dev->flags); > set_bit(R5_ReadError, &dev->flags); > } > } > If there is any bad block in the range of a read, it will fall from cache-bypass > to stripe-cache. With the R5_UPTODATE checking here, we may try to read from this > badblock instead of attempting to calculate the data directly. > > Why do we need the R5_UPTODATE here ? > > From the comment of the commit, > " > We can only treat a known-bad-block like a read-error if > we have the data that belongs in that block > " When there is read error, we would like to write the data again, so the drive can remap the broken sector. This is only possible when we have the data up to date (calculated from other data block and parity). If the data is not up to date, we will need to initiate repair (read other blocks for the stripe, and recover the data). Once this block is up to date, we will go ahead fix the read error on the next iteration. Does this make sense? Thanks, Song > > Would anyone please help to clarify the reason ? > > Thanks in advance > Jianchao >