On Thu, Feb 11 2016, Song Liu wrote: > Signed-off-by: Song Liu <songliubraving@xxxxxx> > Signed-off-by: Shaohua Li <shli@xxxxxx> > --- > drivers/md/md.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index e55e6cf..2997104 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -406,6 +406,8 @@ static void md_submit_flush_data(struct work_struct *ws) > struct mddev *mddev = container_of(ws, struct mddev, flush_work); > struct bio *bio = mddev->flush_bio; > > + if (!bio) > + goto out; > if (bio->bi_iter.bi_size == 0) > /* an empty barrier - all done */ > bio_endio(bio); > @@ -415,6 +417,7 @@ static void md_submit_flush_data(struct work_struct *ws) > } > > mddev->flush_bio = NULL; > +out: > wake_up(&mddev->sb_wait); > } > I don't think this is safe. ->flush_bio is used for two different purposes. One is to carry the bio so it can be passed to ->make_request after the flush. The other is to ensure exclusive access to mddev->flush_work. The first thing md_flush_request() does is wait for ->flush_bio to be NULL. Then it knows that ->flush_work is no longer in use. With your change you could get md_flush_request trying to INIT_WORK ->flush_work while it is already queued. Not good. NeilBrown
Attachment:
signature.asc
Description: PGP signature
