Hi Paul,
At Sat, 20 Feb 2016 21:30:20 -0800,
Paul E. McKenney wrote:
>
> On Sat, Feb 20, 2016 at 05:52:02PM +0900, Hitoshi Mitake wrote:
> > This commit a little bit improves the logic of a writer thread. The
> > new logic lets the writer go back to the original CPU after run_on()
> > the final CPU.
> >
> > Of course it is a trivial improvement and the original two lines
> > pseudocode is beautiful. If it isn't worth to apply, please ignore it.
> >
> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi@xxxxxxxxx>
>
> Hello, Hitoshi,
>
> Thank you for the submission, but each addition of this sort invites
> another. For example, you capture the initial CPU, but suppose that
> the thread was originally bound to a pair of CPUs? Wouldn't you then
> want to restore the binding to that pair of CPUs?
>
> But if you did that, suppose that both of those CPUs went offline
> while going through the for_each_online_cpu() loop? Wouldn't you
> need to instead bind to some CPUs that were actually online?
>
> And suppose that some other CPU changed the binding of the thread
> just after you captured the initial binding? Wouldn't you want to
> reflect that change (give or take CPU hotplug operations) at the
> end of the loop?
>
> And it just gets worse from there. So we need to keep this one
> simple, which means that I cannot accept this patch.
Thanks a lot for your review. I couldn't consider about atomicity of
the CPU state, the affinity of the thread, etc. I assumed the
atomicity is provided and this assumption is wrong.
Again, thanks for your time and review. Your review improved my
understanding of RCU :)
# BTW, how do you think about the change of the figure in the 1st
# patch? Of couse I'm not sure it is worth to apply or not.
Thanks,
Hitoshi
>
> Thanx, Paul
>
> > ---
> > defer/rcuintro.tex | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/defer/rcuintro.tex b/defer/rcuintro.tex
> > index 03d6e12..135922c 100644
> > --- a/defer/rcuintro.tex
> > +++ b/defer/rcuintro.tex
> > @@ -166,8 +166,11 @@ quite complex, a toy implementation is exceedingly simple:
> > \begin{minipage}[t]{\columnwidth}
> > \scriptsize
> > \begin{verbatim}
> > - 1 for_each_online_cpu(cpu)
> > - 2 run_on(cpu);
> > + 1 int orig_cpu = smp_processor_id();
> > + 2 for_each_online_cpu(cpu)
> > + 3 run_on(cpu);
> > + 4 if (orig_cpu != smp_processor_id())
> > + 5 run_on(orig_cpu);
> > \end{verbatim}
> > \end{minipage}
> > \vspace{5pt}
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe perfbook" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
--
To unsubscribe from this list: send the line "unsubscribe perfbook" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
