From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680890674; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=viOP9MeRFNTOj2kIVCqFRA1TknEueWIg7cE4M74NtKs=; b=Disyl2H0aYgXLv0jOSGSWFBK9eepoGIHGddnZnVD/7jOruAVJdr6hWf/nTDfF/e041 wDn5G6i9auUPPR2jSjVKDBuy/r5L4jYs8n4XJMZShWUQh+306Ztos6TWALwPwKGSHHSS KeA5Kf6YPI1RTfRKdSBzDxSUjkeJOaokQQLa8k3rH+R/3UN6wLw2BctL7b+w4lnnWUuU 2SzlJMnz0+Ou9vEmk7hBt5Ofoa2S9KMHyR6iNPY1Y39LmEcLw6+JKD25xGzxS8Lb3nbO B5j1H8Eh08P1td7aU2YIHeUcs0Zm/Sy/6GvZKY8Y0G/wMaH1XEQI+Txtrv7avS6csCoa UOtw== Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.60.0.2.21\)) Subject: Re: [PATCH] CodeSamples/count: Remove unnecessary memory barriers From: Alan Huang In-Reply-To: <20230407175813.1334028-1-mmpgouride@gmail.com> Date: Sat, 8 Apr 2023 02:04:24 +0800 Content-Transfer-Encoding: quoted-printable Message-Id: <81ADBFE2-3576-48EC-A892-B49A5C265487@gmail.com> References: <20230407175813.1334028-1-mmpgouride@gmail.com> To: paulmck@kernel.org, akiyks@gmail.com Cc: perfbook@vger.kernel.org List-ID: Hi Paul and Akira, This is the patch v3, I forgot to add a version tag=E2=80=A6 And I think it may be better to add a question in a subsequent path. Thanks, Alan > 2023=E5=B9=B44=E6=9C=888=E6=97=A5 =E4=B8=8A=E5=8D=881:58=EF=BC=8CAlan = Huang =E5=86=99=E9=81=93=EF=BC=9A >=20 > In count_lim_sig.c, there is only one ordering required, that is > writing to counter happens before setting theft to THEFT_READY in > add_count/sub_count's fast path. Therefore, partial memory barrier > will suffice. >=20 > Signed-off-by: Alan Huang > --- > CodeSamples/count/count_lim_sig.c | 10 +++------- > count/count.tex | 12 ++++++------ > 2 files changed, 9 insertions(+), 13 deletions(-) >=20 > diff --git a/CodeSamples/count/count_lim_sig.c = b/CodeSamples/count/count_lim_sig.c > index 59da8077..c2f61197 100644 > --- a/CodeSamples/count/count_lim_sig.c > +++ b/CodeSamples/count/count_lim_sig.c > @@ -56,12 +56,10 @@ static void flush_local_count_sig(int unused) = //\lnlbl{flush_sig:b} > { > if (READ_ONCE(theft) !=3D THEFT_REQ) = //\lnlbl{flush_sig:check:REQ} > return; = //\lnlbl{flush_sig:return:n} > - smp_mb(); //\lnlbl{flush_sig:mb:1} > WRITE_ONCE(theft, THEFT_ACK); = //\lnlbl{flush_sig:set:ACK} > if (!counting) { = //\lnlbl{flush_sig:check:fast} > - WRITE_ONCE(theft, THEFT_READY); = //\lnlbl{flush_sig:set:READY} > + smp_store_release(&theft, THEFT_READY); = //\lnlbl{flush_sig:set:READY} > } > - smp_mb(); > } //\lnlbl{flush_sig:e} >=20 > static void flush_local_count(void) = //\lnlbl{flush:b} > @@ -125,8 +123,7 @@ int add_count(unsigned long delta) = //\lnlbl{b} > WRITE_ONCE(counting, 0); = //\lnlbl{clearcnt} > barrier(); = //\lnlbl{barrier:3} > if (READ_ONCE(theft) =3D=3D THEFT_ACK) { = //\lnlbl{check:ACK} > - smp_mb(); //\lnlbl{mb} > - WRITE_ONCE(theft, THEFT_READY); //\lnlbl{READY} > + smp_store_release(&theft, THEFT_READY); = //\lnlbl{READY} > } > if (fastpath) > return 1; = //\lnlbl{return:fs} > @@ -164,8 +161,7 @@ int sub_count(unsigned long delta) > WRITE_ONCE(counting, 0); > barrier(); > if (READ_ONCE(theft) =3D=3D THEFT_ACK) { > - smp_mb(); > - WRITE_ONCE(theft, THEFT_READY); > + smp_store_release(&theft, THEFT_READY); > } > if (fastpath) > return 1; > diff --git a/count/count.tex b/count/count.tex > index 8ab67e2e..4c139d63 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, with the release store ensuring any change to counter = in > +the fastpath happens before the change of theft to READY\@. > \end{fcvref} >=20 > \begin{listing} > @@ -2595,9 +2594,10 @@ 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 > -READY also sees the effects of \clnref{add:f}. > +state-change to READY, and, if so, \clnref{READY} changes theft to > +READY with the release store ensuring that > +any CPU that sees the READY state also sees the effects > +of \clnref{add:f}. > If the fastpath addition at \clnref{add:f} was executed, then > \clnref{return:fs} returns > success. > --=20 > 2.34.1 >=20