On Fri, Oct 06, 2023 at 03:47:31PM +0300, Vitaliy Makkoveev wrote:
> On Fri, Oct 06, 2023 at 02:14:52PM +0200, Alexander Bluhm wrote:
> > > @@ -718,11 +743,13 @@ softclock(void *arg)
> > >                   softclock_process_tick_timeout(to, new);
> > >   }
> > >   tostat.tos_softclocks++;
> > > - needsproc = !CIRCQ_EMPTY(&timeout_proc);
> > > - mtx_leave(&timeout_mutex);
> > > -
> > > - if (needsproc)
> > > + if (!CIRCQ_EMPTY(&timeout_proc))
> > >           wakeup(&timeout_proc);
> > > +#ifdef MULTIPROCESSOR
> > > + if(!CIRCQ_EMPTY(&timeout_proc_mpsafe))
> > > +         wakeup(&timeout_proc_mpsafe);
> > > +#endif
> > > + mtx_leave(&timeout_mutex);
> > >  }
> > >  
> > >  void
> > 
> > Was there a good reason that wakeup() did run without mutex?
> > Do we really want to change this?
> > 
> 
> I dont understand you. Original code does wakeup() outside mutex. I
> moved wakeup() under mutex. You want to move it back?

I just wanted to know why you moved it.

Now I see.  You use msleep_nsec() with timeout_mutex.  Putting
wakeup in mutex ensures that you don't miss it.

Nitpick: timeoutmp_proc should be timeout_procmp.  timeout_ is the
prefix in this file.  mp suffix is easier to see at the end.

>+       if (kthread_create(softclockmp_thread, NULL, NULL, "softclockm"))
"softclockm" -> "softclockmp"

OK bluhm@, but let's wait for cheloha@ and see what he thinks

Reply via email to