Re: [PATCH] multipathd: fix mpp->hwe handling on path removal

From: Martin Wilck <mwilck@suse.com>
To: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: dm-devel@redhat.com
Subject: Re: [PATCH] multipathd: fix mpp->hwe handling on path removal
Date: Mon, 01 Oct 2018 16:54:09 +0200	[thread overview]
Message-ID: <f462a9c6b25121485bd19fb8a31c2502306e2414.camel@suse.com> (raw)
In-Reply-To: <CABr-GndXjnbDqp8AYFEoPZf8ckkFe-R2SPL4QBrCR+ujUKaj3g@mail.gmail.com>

Hello Christophe,

On Mon, 2018-07-02 at 12:05 +0200, Christophe Varoqui wrote:
> Right, done.
> The leftover from your previous v3 patchset is merged too.
> Thanks,
> Christophe.

I think in that mail from July 2nd, you referred to the following patch
set which had been acked by Ben on June 25th 
(https://www.redhat.com/archives/dm-devel/2018-June/msg00367.html):

 * [PATCH v3 00/10] libmpathpersist fixes and some more
 * [PATCH v3 01/10] libmpathpersist: remove duplicate test in 
 * [PATCH v3 02/10] libmpathpersist: fix typo in
 * [PATCH v3 03/10] libmpathpersist: fix stack overflow in
 * [PATCH v3 04/10] libmultipath: add (get|put)_unaligned_be64
 * [PATCH v3 05/10] multipath-tools/tests: add tests for
 * [PATCH v3 06/10] libmpathpersist: fix byte swapping for big endian
 * [PATCH v3 07/10] (lib)mpathpersist: use O_RDONLY file descriptors
 * [PATCH v3 08/10] libmultipath: fix gcc 8.1 "truncated output"
 * [PATCH v3 09/10] multipathd: fix buffer size in cli_getprkey()
 * [PATCH v3 10/10] libmultipath: avoid error messages from RDAC check

I just double-checked, and it seems that out of these, only patch 9 and
10 got applied to your tree. Did you have an issue with the others, or
was it just an oversight?

Regards
Martin

PS: I'll send a review or Ben's late big series very soon.

> 
> On Mon, Jul 2, 2018 at 11:33 AM Martin Wilck <mwilck@suse.com> wrote:
> > Christophe,
> > 
> > without this patch, current upstream multipath _crashes_ on path
> > removal. Please apply it. If reviewers disagree with this fix, I
> > can
> > post updates later.
> > 
> > Martin
> > 
> > On Mon, 2018-06-25 at 17:10 +0200, Martin Wilck wrote:
> > > In my previous patch f0462f0c8338, I overlooked that during path
> > > removal,
> > > the path that mpp->hwe references may be removed and and thus
> > mpp-
> > > >hwe
> > > may become stale. Fix it.
> > > 
> > > Fixes: f0462f0c8338 "libmultipath: use vector for for pp->hwe and
> > mp-
> > > >hwe"
> > > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > > ---
> > >  multipathd/main.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/multipathd/main.c b/multipathd/main.c
> > > index 72f06b56..cc493c18 100644
> > > --- a/multipathd/main.c
> > > +++ b/multipathd/main.c
> > > @@ -1076,6 +1076,14 @@ ev_remove_path (struct path *pp, struct
> > > vectors * vecs, int need_do_map)
> > >                               mpp->alias);
> > >                       goto fail;
> > >               }
> > > +
> > > +             /*
> > > +              * Make sure mpp->hwe doesn't point to freed memory
> > > +              * We call extract_hwe_from_path() below to restore
> > > mpp->hwe
> > > +              */
> > > +             if (mpp->hwe == pp->hwe)
> > > +                     mpp->hwe = NULL;
> > > +
> > >               if ((i = find_slot(mpp->paths, (void *)pp)) != -1)
> > >                       vector_del_slot(mpp->paths, i);
> > >  
> > > @@ -1109,6 +1117,9 @@ ev_remove_path (struct path *pp, struct
> > vectors
> > > * vecs, int need_do_map)
> > >                        */
> > >               }
> > >  
> > > +             if (mpp->hwe == NULL)
> > > +                     extract_hwe_from_path(mpp);
> > > +
> > >               if (setup_map(mpp, params, PARAMS_SIZE, vecs)) {
> > >                       condlog(0, "%s: failed to setup map for"
> > >                               " removal of path %s", mpp->alias,
> > > pp->dev);
> > 
> > --
> > dm-devel mailing list
> > dm-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/dm-devel

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel