On Mon, 2018-03-19 at 11:25 -0500, Benjamin Marzinski wrote:
> On Mon, Mar 19, 2018 at 01:59:24PM +0100, Martin Wilck wrote:
> > On Fri, 2018-03-16 at 16:31 -0500, Benjamin Marzinski wrote:
> > > diff --git a/multipathd/waiter.c b/multipathd/waiter.c
> > > index cb9708b..e894294 100644
> > > --- a/multipathd/waiter.c
> > > +++ b/multipathd/waiter.c
> > > @@ -23,6 +23,7 @@
> > > #include "waiter.h"
> > >
> > > pthread_attr_t waiter_attr;
> > > +struct mutex_lock waiter_lock = { .mutex =
> > > PTHREAD_MUTEX_INITIALIZER
> > > };
> > >
> > > static struct event_thread *alloc_waiter (void)
> > > {
> > > @@ -59,8 +60,12 @@ void stop_waiter_thread (struct multipath
> > > *mpp,
> > > struct vectors *vecs)
> > > mpp->waiter);
> > > thread = mpp->waiter;
> > > mpp->waiter = (pthread_t)0;
> > > - pthread_cancel(thread);
> > > + pthread_cleanup_push(cleanup_lock, &waiter_lock);
> > > + lock(&waiter_lock);
> > > + pthread_testcancel();
> >
> > What's the purpose of this pthread_testcancel() call?
> >
>
> It's not necessary. I feel like in general, after you get done
> waiting
> on a lock, it's good form to see if you've been cancelled while you
> were
> waiting. However, it's really unlikely that a thread will be waiting
> here long, and this pthread_cancel isn't protecting it form accessing
> any freed data. If you think it's likely to confuse people, I have no
> problem with removing it.
My thinking was: this thread is just about to cancel another thread. If
it's cancelled itself just before doing that, the other one won't be
cancelled. I find it hard to imagine conditions where that would be the
right thing. So if at all, I'd call pthread_testcancel() after
releasing the lock. But probably it's best just to remove it here.
Regards,
Martin
--
Dr. Martin Wilck <mwilck@xxxxxxxx>, 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
