Thanks for the suggestion!
I’ll send patch v3 later.
Thanks,
Alan
> 2023年4月7日 10:14，Akira Yokosawa <akiyks@xxxxxxxxx> 写道：
>
> On Fri, 7 Apr 2023 00:29:21 +0800, Alan Huang wrote:
>> Hi Akira,
>>
>>> Do you have a good reasoning why the pair of smp_mb()s can be safely
>>> replaced with the single smp_store_release() ?
>>
>> When the signal handler is running, there is only one thread waiting on the while loop in flush_local_count,
>> which means we only need to synchronize with that thread, and that thread only cares the THEFT_READY status,
>> we just need to make sure after that thread seeing theft equals THEFT_READY, it can also see the changes to counter.
>>
>> And because the signal handler is running on the interrupted thread, we don’t need any memory barrier.
>
> OK, now I'm convinced. You are right!
>
> Let's see your changes in corresponding text:
>
>> diff --git a/count/count.tex b/count/count.tex
>> index 80ada104..899ea7e9 100644
>> --- a/count/count.tex
>> +++ b/count/count.tex
>> @@ -2425,12 +2425,11 @@ handler used in the theft process.
>> \Clnref{check:REQ,return:n} check to see if
>> the \co{theft} state is REQ, and, if not
>> returns without change.
>> -\Clnref{mb:1} executes a \IX{memory barrier} to ensure that the sampling
>> -of the theft variable happens before any change to that variable.
>> \Clnref{set:ACK} sets the \co{theft} state to ACK, and, if
>> \clnref{check:fast} sees that
>> this thread's fastpaths are not running, \clnref{set:READY} sets the \co{theft}
>> -state to READY\@.
>> +state to READY, the release store here is to ensure that the change to counter
>> +in fast path happens before changing the theft varibale to READY\@.
> the fastpath variable
>
> I can't parse this sentence grammatically.
>
> This sentence is rendered as follows (with minor nits fixed):
>
> Line 13 sets the theft state to ACK, and, if line 14 sees that this
> thread's fastpaths are not running, line 15 sets the theft state
> to READY, the release store here is to ensure that the change to
> counter in the fastpath happens before changing the theft variable
> to READY.
>
> How about this?
>
> Line 13 sets the theft state to ACK, and, if line 14 sees that this
> thread's fastpaths are not running, line 15 sets the theft state
> to READY, with the release store ensuring any change to counter in
> the fastpath happens before the change of theft to READY.
>
>
> Also, I'd like to see your reasoning above mentioned somewhere around
> here. Might worth a Quick Quiz.
>
> Question:
> Doesn't flush_local_count_sig() need a full memory barrier?
>
>> \end{fcvref}
>>
>> \begin{listing}
>> @@ -2595,8 +2594,8 @@ handlers to undertake theft.
>> \Clnref{barrier:3} again disables compiler reordering, and then
>> \clnref{check:ACK}
>> checks to see if the signal handler deferred the \co{theft}
>> -state-change to READY, and, if so, \clnref{mb} executes a memory
>> -barrier to ensure that any CPU that sees \clnref{READY} setting state to
>> +state-change to READY, and, if so, the release store at \clnref{READY}
>> +is to ensure that any CPU that sees \clnref{READY} setting state to
>> READY also sees the effects of \clnref{add:f}.
>
> Again, this looks to me incomplete in grammatical structure.
>
> This sentence is rendered as follows:
>
> Line 14 again disables compiler reordering, and then line 15 checks
> to see if the signal handler deferred the theft state-change to READY,
> and, if so, the release store at line 16 is to ensure that any CPU
> that sees line 16 setting state to READY also sees the effects of
> line 9.
>
> How about something like this?
>
> Line 14 again disables compiler reordering, and then line 15 checks
> to see if the signal handler deferred the theft state-change to READY,
> and, if so, line 16 changes theft to READY with the release store
> ensuring that any CPU that sees the READY state also sees the effects
> of line 9.
>
> Paul might have better ideas. ;-)
>
>
>> If the fastpath addition at \clnref{add:f} was executed, then
>> \clnref{return:fs} returns
>
> Can you post a v3 with whatever text-side updates you come up with?
>
> Thanks, Akira
>
>>
>> Please correct me if I'm wrong.
>>
>> Thanks,
>> Alan
