From: Maxime Coquelin > > From: Maxime Coquelin > >> On 1/14/21 4:23 PM, Matan Azrad wrote: > >>> > >>> > >>> From: Maxime Coquelin > >>>> On 1/14/21 2:09 PM, Matan Azrad wrote: > >>>>> > >>>>> > >>>>> From: Maxime Coquelin > >>>>>> Hi Matan, > >>>>>> > >>>>>> On 1/14/21 12:49 PM, Matan Azrad wrote: > >>>>>>> Hi Maxime and David > >>>>>>> > >>>>>>> Thank you for Review. > >>>>>>> > >>>>>>> From: David Marchand > >>>>>>>> On Fri, Jan 8, 2021 at 9:48 AM David Marchand > >>>>>>>> <david.march...@redhat.com> wrote: > >>>>>>>>>> I wonder if it would be possible and cleaner to disable > >>>>>>>>>> cancellation on the thread while the mutex is held? > >>>>>>> > >>>>>>> Yes, we can cause thread to return by some global variable sync. > >>>>>>> It is the same logic. > >>>>>> > >>>>>> No, that was not my suggestion. My suggestion is to block the > >>>>>> thread cancellation while in the critical section, using > >> pthread_setcancelstate(). > >>>>> > >>>>> Yes, Generally it is better to let the thread control his > >>>>> cancellation, either > >>>> cancel itself or enabling\disabling cancellations. > >>>>> > >>>>> I don't see a reason to wait for the thread in current logic - the > >>>>> critical section > >>>> is not important to be completed here. > >>>> > >>>> The reason I see is there are quite a few things done in this > >>>> critical section. And if tomorrow someone add new things in it, he > >>>> may not know the thread can be cancelled at any time, which could > >>>> cause > >> hard to debug issues. > >>> > >>> As I said, here it is not needed, this thread designed just to cause > >>> guest > >> notifications. > >>> > >>> The optional future developer mistake can be done also outside the > >>> critical > >> section in in any other place - we cannot protect it. > >>> > >>> The design choice is to close the thread fast. > >> > >> But why is it so urgent that it cannot been stopped cleanly? > >> I don't think it would add seconds delay by doing it in a clean way. > > > > We have system calls there per queue. > > No need this optional delay just because of mutex cleaning. > > OK, up to you... > > And what about the timer lock?
Existing code initiates it before reusing... Thanks. > > > > > > >> Thanks, > >> Maxime > >> > >>>>> We just want to close the thread and to clean the mutex. > >>>>> > >>>>>>>>> +1 > >>>>>>>> > >>>>>>>> IEEE Std 1003.1-2001/Cor 2-2004, item XBD/TC2/D6/26 is applied, > >>>>>>>> adding pthread_t to the list of types that are not required to > >>>>>>>> be arithmetic types, thus allowing pthread_t to be defined as a > structure. > >>>>>>>> > >>>>>>>> It would be better to leave pthread_t alone and not interpret it: > >>>>>>>> > >>>>>>>> if (priv->timer_tid) { > >>>>>>>> pthread_cancel(priv->timer_tid); > >>>>>>>> pthread_join(priv->timer_tid, &status); } > >>>>>>>> priv->timer_tid = 0; > >>>>>>> > >>>>>>> > >>>>>>> I'm not sure why you think it is better in this specific case. > >>>>>>> The cancellation will close the thread in faster way, no need to > >>>>>>> wait for the > >>>>>> thread to close itself. > >>>>>>> > >>>>>>> > >>>>>>>> > >>>>>>>> -- > >>>>>>>> David Marchand > >>>>>>> > >>>>> > >>> > >