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?
--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]
>
