Hi Song,
Thanks a lot for your reply.
On 8/26/19 10:07 PM, Song Liu wrote:
Hi Guoqing,
Sorry for the delay.
On Mon, Aug 26, 2019 at 6:46 AM Guoqing Jiang
<guoqing.jiang@xxxxxxxxxxxxxxx> wrote:
[...]
Maybe it makes sense to remove the checking of STRIPE_ACTIVE just like
commit 550da24f8d62f
("md/raid5: preserve STRIPE_PREREAD_ACTIVE in break_stripe_batch_list").
@@ -4606,8 +4607,7 @@ static void break_stripe_batch_list(struct
stripe_head *head_sh,
list_del_init(&sh->batch_list);
- WARN_ONCE(sh->state & ((1 << STRIPE_ACTIVE) |
- (1 << STRIPE_SYNCING) |
+ WARN_ONCE(sh->state & ((1 << STRIPE_SYNCING) |
(1 << STRIPE_REPLACED) |
(1 << STRIPE_DELAYED) |
(1 << STRIPE_BIT_DELAY) |
This part looks good to me.
@@ -4626,6 +4626,7 @@ static void break_stripe_batch_list(struct
stripe_head *head_sh,
set_mask_bits(&sh->state, ~(STRIPE_EXPAND_SYNC_FLAGS |
(1 <<
STRIPE_PREREAD_ACTIVE) |
+ (1 << STRIPE_ACTIVE) |
(1 << STRIPE_DEGRADED) |
(1 <<
STRIPE_ON_UNPLUG_LIST)),
head_sh->state & (1 << STRIPE_INSYNC));
But I think we should not clear STRIPE_ACTIVE here. It should be
cleared at the end of handle_stripe().
Yes, actually "clear_bit_unlock(STRIPE_ACTIVE, &sh->state)" is the last
line in handle_stripe. So we only need to do one line change like.
@@ -4606,8 +4607,7 @@ static void break_stripe_batch_list(struct
stripe_head *head_sh,
list_del_init(&sh->batch_list);
- WARN_ONCE(sh->state & ((1 << STRIPE_ACTIVE) |
- (1 << STRIPE_SYNCING) |
+ WARN_ONCE(sh->state & ((1 << STRIPE_SYNCING) |
(1 << STRIPE_REPLACED) |
(1 << STRIPE_DELAYED) |
(1 << STRIPE_BIT_DELAY) |
I will send formal patch if you are fine with above, thanks again.
BR,
Guoqing
