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@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to