On Fri, 2018-02-09 at 11:26 -0600, Benjamin Marzinski wrote:
> On Fri, Feb 09, 2018 at 04:15:34PM +0000, Bart Van Assche wrote:
> > On Thu, 2018-02-08 at 17:56 -0600, Benjamin Marzinski wrote:
> > > static void cleanup_func(void *data)
> > > {
> > > - int holders;
> > > + int running, holders;
> > > struct tur_checker_context *ct = data;
> > > - pthread_spin_lock(&ct->hldr_lock);
> > > - ct->holders--;
> > > - holders = ct->holders;
> > > - ct->thread = 0;
> > > - pthread_spin_unlock(&ct->hldr_lock);
> > > +
> > > + running = uatomic_xchg(&ct->running, 0);
> > > + holders = uatomic_sub_return(&ct->holders, 1);
> > > if (!holders)
> > > cleanup_context(ct);
> > > + if (!running)
> > > + pause();
> > > }
> >
> > Hello Ben,
> >
> > Why has the pause() call been added? I think it is safe to call pthread_cancel()
> > for a non-detached thread that has finished so I don't think that pause() call
> > is necessary.
>
> Martin objected to having the threads getting detached as part of
> cancelling them (I think. I'm a little fuzzy on what he didn't like).
> But he definitely said he preferred the thread to start detached, so in
> this version, it does. That's why we need the pause(). If he's fine with
> the threads getting detached later, I will happily replace the pause()
> with
>
> if (running)
> pthread_detach(pthread_self());
>
> and add pthread_detach(ct->thread) after the calls to
> pthread_cancel(ct->thread). Otherwise we need the pause() to solve your
> original bug.
Ah, thanks, I had overlooked that the tur checker detaches the checker thread. Have
you considered to add a comment above the pause() call that explains the purpose of
that call?
Thanks,
Bart.
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
