On Tue, Nov 05 2013 at 10:56am -0500,
Alasdair G Kergon <agk@xxxxxxxxxx> wrote:
> On Thu, Oct 31, 2013 at 10:48:13AM -0400, Mike Snitzer wrote:
> > --- linux.orig/drivers/md/dm-mpath.c
> > +++ linux/drivers/md/dm-mpath.c
>
> > + unsigned pg_init_disabled:1; /* pginit is currently not allowed */
>
> > + if (m->pg_init_required && !m->pg_init_in_progress && pgpath &&
> > + !m->pg_init_disabled)
>
> So we're accepting that pg_init might be both "disabled" and "required"
> simultaneously (otherwise we wouldn't need this test)?
Yes, needs to be independent of pg_init_required.
> > @@ -942,10 +944,20 @@ static void multipath_wait_for_pg_init_c
> > static void flush_multipath_work(struct multipath *m)
> > {
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&m->lock, flags);
> > + m->pg_init_disabled = 1;
> > + spin_unlock_irqrestore(&m->lock, flags);
> > +
> > flush_workqueue(kmpath_handlerd);
> > multipath_wait_for_pg_init_completion(m);
>
> So this implies pg_init could even still be running while it is disabled?
pg_init_disabled doesn't allow pg_init to be initiated once set (even if
pg_init_required is). Hence pg_init is disabled.
> If the logic is correct, then I find the new variable name quite confusing and
> think it should be changed, or as a minimum have inline comments explaining
> the apparent contradictions!
I'm not seeing a contradiction.
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
