Re: [PATCH 10/10] memorder: Fix wrong primitive names in a quick quizz

Hi Akira,

On Sun, 24 Sep 2023 09:02:53 +0900 Akira Yokosawa <[email protected]> wrote:

> Hi,
> 
> On 2023/09/24 1:32, SeongJae Park wrote:
> > From: SeongJae Park <[email protected]>
> > 
> > memorder.tex explains READ_ONCE() is having smp_mb(), but the following
> > quick quizz asks why it has mb() instead of rmb().  Fix the wrong names
> > in the quick quizz.
> > 
> > Signed-off-by: SeongJae Park <[email protected]>
> > ---
> >  memorder/memorder.tex | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/memorder/memorder.tex b/memorder/memorder.tex
> > index 5c978fbe..b3acc14a 100644
> > --- a/memorder/memorder.tex
> > +++ b/memorder/memorder.tex
> > @@ -5491,7 +5491,7 @@ Alpha is the only CPU whose \co{READ_ONCE()} includes an \co{smp_mb()}.
> >  \QuickQuizSeries{%
> >  \QuickQuizB{
> >  	Why does Alpha's \co{READ_ONCE()} include an
> > -	\co{mb()} rather than \co{rmb()}?
> > +	\co{smp_mb()} rather than \co{smp_rmb()}?
> 
> Definition in Linux kernel's arch/alpha/include/asm/rwonce.h reads:
> 
> #define __READ_ONCE(x)							\
> ({									\
> 	__unqual_scalar_typeof(x) __x =					\
> 		(*(volatile typeof(__x) *)(&(x)));			\
> 	mb();								\
> 	(typeof(x))__x;							\
> })
> 
> So I don't think this change is needed.
> 
> As this quiz is talking about Alpha ISA, how about rewording the
> quiz to:
> 
>     Why does Alpha's \co{READ_ONCE()} include an
>     \co{mb} instruction rather than a \co{rmb}?
> 
>         Thanks, Akira
> 
> >  }\QuickQuizAnswerB{
> >  	Alpha has only \co{mb} and \co{wmb} instructions,
> >  	so \co{smp_rmb()} would be implemented by the Alpha \co{mb}
> > -- 
> > 2.17.1

That makes sense.  Thank you for nice review and suggestion!  I will make the
second version of this patch following your comment.


Thanks,
SJ