On Mon, Jan 6, 2020 at 1:34 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxxxxxxxx> wrote: > > > > On 1/3/20 11:56 PM, Song Liu wrote: > > On Fri, Jan 3, 2020 at 11:48 AM Song Liu <liu.song.a23@xxxxxxxxx> wrote: > >> On Fri, Jan 3, 2020 at 5:56 AM <jgq516@xxxxxxxxx> wrote: > >>> From: Guoqing Jiang <guoqing.jiang@xxxxxxxxxxxxxxx> > >>> > >> [...] > >>> --- > >>> drivers/md/raid5.c | 4 +++- > >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c > >>> index d4d3b67ffbba..70ef2367fa64 100644 > >>> --- a/drivers/md/raid5.c > >>> +++ b/drivers/md/raid5.c > >>> @@ -5481,7 +5481,9 @@ static void release_stripe_plug(struct mddev *mddev, > >>> INIT_LIST_HEAD(cb->temp_inactive_list + i); > >>> } > >>> > >>> - if (!test_and_set_bit(STRIPE_ON_UNPLUG_LIST, &sh->state)) > >>> + if (!atomic_read(&sh->count) == 0 && > >> "!atomic_read(&sh->count) == 0" is confusing. Do you actually mean > >> "atomic_read(&sh->count) == 0"? > > If "atomic_read(&sh->count) == 0" is true, which means the sh is handled by > do_release_stripe, so the sh could be added to other lists as I said in > header, > so I think we should not add the sh to cb->list for this case. IOW, the > sh could > be added to callback list only if it's count is not '0', no? I see. I guess it is cleaner with atomic_read(&sh->count) != 0 or atomic_read(&sh->count) > 0 ? Thanks, Song