On Sat, 22 Sep 2012 10:22:58 +0800 "Jianpeng Ma" <majianpeng@xxxxxxxxx> wrote:
> In func analyse_stripe, it added to_read/to_write by
> sh->dev[i]->toread/towrite.
> If stripe failed, in func handle_failed_stripe it decreased
> to_read/to_write also by sh->dev[i]->toread/towrite.
> But between func analyse_stripe and handle_failed_stripe, toread/towrite
> can change.So the to_read/to_write maybe less zero.
> For example, if to_write was less zero,it may introduce a bug in func async_xor:
> 'BUG_ON(src_cnt <= 1);'.
> So after handle_failed_stripe,it should use 'to_read/to_write > 0' instead
> of judging 'to_read/to_write'.
>
> Signed-off-by: Jianpeng Ma <majianpeng@xxxxxxxxx>
> ---
> drivers/md/raid5.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index a56aa5b..554ca9e 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -2641,7 +2641,7 @@ static int fetch_block(struct stripe_head *sh, struct stripe_head_state *s,
> (s->failed >= 2 && fdev[1]->toread) ||
> (sh->raid_conf->level <= 5 && s->failed && fdev[0]->towrite &&
> !test_bit(R5_OVERWRITE, &fdev[0]->flags)) ||
> - (sh->raid_conf->level == 6 && s->failed && s->to_write))) {
> + (sh->raid_conf->level == 6 && s->failed && s->to_write > 0))) {
> /* we would like to get this block, possibly by computing it,
> * otherwise read it if the backing disk is insync
> */
> @@ -3471,8 +3471,8 @@ static void handle_stripe(struct stripe_head *sh)
> * parity, or to satisfy requests
> * or to load a block that is being partially written.
> */
> - if (s.to_read || s.non_overwrite
> - || (conf->level == 6 && s.to_write && s.failed)
> + if (s.to_read > 0 || s.non_overwrite
> + || (conf->level == 6 && s.to_write > 0 && s.failed)
> || (s.syncing && (s.uptodate + s.compute < disks))
> || s.replacing
> || s.expanding)
> @@ -3519,7 +3519,7 @@ static void handle_stripe(struct stripe_head *sh)
> * 2/ A 'check' operation is in flight, as it may clobber the parity
> * block.
> */
> - if (s.to_write && !sh->reconstruct_state && !sh->check_state)
> + if (s.to_write > 0 && !sh->reconstruct_state && !sh->check_state)
> handle_stripe_dirtying(conf, sh, &s, disks);
>
> /* maybe we need to check and possibly fix the parity for this stripe
Thanks.
However I would prefer to fix this by not decrementing the counters in
handle_failed_stripe. There is no harm in the counters being a little bigger
than they should be.
I have queued a patch to do this.
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
