On Wed, Apr 12, 2023 at 07:32:45AM -0400, Elad Lahav wrote:
> I may be missing something, but why do you need memory barriers at all
> in the signal handler? At least on the systems I know both setting up
> the signal handler and restoring the context require kernel calls. On
> x86 this doesn't matter as it is strongly ordered, but on ARM the
> kernel calls provide full synchronization points.
> Are there architectures/OS combinations where that is not true?
It is quite possible that there is still more memory ordering in this
code than is absolutely required.
I would have some difficulty forcing myself to rely on implicit ordering
due to signal entry and exit, but then again, I have also some difficulty
forcing myself to rely on signals. ;-)
But if there is some signal entry/exit guarantee in (say) POSIX, we should
be OK relying on it.
Any thoughts on interrupt handlers in kernels?
Thanx, Paul
> --Elad
>
> On Tue, Apr 11, 2023 at 12:43 PM Alan Huang <mmpgouride@xxxxxxxxx> wrote:
> >
> > The signal-theft counter reminds me of the signal-based user space RCU implementation, the signal hander there
> > has two full memory barriers around it, how about mentioning that a little?
> >
> > Thanks,
> > Alan
> >
> >
> >
> > > 2023年4月11日 上午3:06，Paul E. McKenney <paulmck@xxxxxxxxxx> 写道：
> > >
> > > On Sun, Apr 09, 2023 at 09:11:58AM +0900, Akira Yokosawa wrote:
> > >> Hi,
> > >>
> > >> On Sat, 8 Apr 2023 02:04:24 +0800, Alan Huang wrote:
> > >>> Hi Paul and Akira,
> > >>>
> > >>> This is the patch v3, I forgot to add a version tag…
> > >>>
> > >>> And I think it may be better to add a question in a subsequent path.
> > >>
> > >> So I'm looking at Quick Quizzes on count_lim_sig.c.
> > >> It looks to me Quick Quiz 5.50 lost its context due to the
> > >> removal of smp_mb() and the use of smp_store_release().
> > >>
> > >> A band-aide fix would be to change the quiz to:
> > >>
> > >> In Listing 5.18's function flush_local_count_sig(), why
> > >> are there READ_ONCE(), WRITE_ONCE(), and smp_store_release()
> > >> wrappers around the uses of the theft per-thread variable?
> > >>
> > >> , and to adjust the line count in its Answer.
> > >>
> > >> However, smp_store_release() is by no means a simple wrapper.
> > >> So I'm wondering if it is worthwhile to keep this quiz.
> > >>
> > >> Paul, how about replacing the quiz with the new quiz asking:
> > >>
> > >> In Listing 5.18, doesn't flush_local_count_sig() need
> > >> a stronger memory barrier or two?
> > >>
> > >> , and picking Alan's reasoning in the answer?
> > >
> > > How about something like this?
> > >
> > > Thanx, Paul
> > >
> > > ------------------------------------------------------------------------
> > >
> > > diff --git a/count/count.tex b/count/count.tex
> > > index 2aade053..ccc2e839 100644
> > > --- a/count/count.tex
> > > +++ b/count/count.tex
> > > @@ -2441,28 +2441,11 @@ the fastpath happens before this change of \co{theft} to READY\@.
> > >
> > > \QuickQuiz{
> > > In \cref{lst:count:Signal-Theft Limit Counter Value-Migration Functions}'s
> > > - function \co{flush_local_count_sig()}, why are there
> > > - \co{READ_ONCE()} and \co{WRITE_ONCE()} wrappers around
> > > - the uses of the
> > > - \co{theft} per-thread variable?
> > > + doesn't \co{flush_local_count_sig()} need stronger memory barriers?
> > > }\QuickQuizAnswer{
> > > - \begin{fcvref}[ln:count:count_lim_sig:migration:flush_sig]
> > > - The first one (on \clnref{check:REQ}) can be argued to be unnecessary.
> > > - The last two (\clnref{set:ACK,set:READY}) are important.
> > > - If these are removed, the compiler would be within its rights
> > > - to rewrite \clnrefrange{set:ACK}{set:READY} as follows:
> > > - \end{fcvref}
> > > -
> > > -\begin{VerbatimN}[firstnumber=14]
> > > -theft = THEFT_READY;
> > > -if (counting) {
> > > - theft = THEFT_ACK;
> > > -}
> > > -\end{VerbatimN}
> > > -
> > > - This would be fatal, as the slowpath might see the transient
> > > - value of \co{THEFT_READY}, and start stealing before the
> > > - corresponding thread was ready.
> > > + No, that \co{smp_store_release()} suffices because this code
> > > + communicates only with \co{flush_local_count()}, and there is
> > > + no need for store-to-load ordering.
> > > }\QuickQuizEnd
> > >
> > > \begin{fcvref}[ln:count:count_lim_sig:migration:flush]
> >
