On Wed, Dec 02, 2020 at 07:44:02PM +0100, Anton Lindqvist wrote:
> On Wed, Dec 02, 2020 at 11:41:04AM -0300, Martin Pieuchot wrote:
> > On 02/12/20(Wed) 17:27, Jonathan Matthew wrote:
> > > On Tue, Dec 01, 2020 at 02:35:18PM -0300, Martin Pieuchot wrote:
> > > > On 01/12/20(Tue) 15:30, Claudio Jeker wrote:
> > > > > [...]
> > > > > Did you run a make build with that smr_barrier() in it and checked
> > > > > that it
> > > > > does not cause a slow down? I am sceptical, smr_barrier() is a very
> > > > > slow
> > > > > construct which introduces large delays and should be avoided whenever
> > > > > possible.
> > > >
> > > > I did build GENERIC.MP multiple times on a 4CPU sparc64 with the diff
> > > > below, without noticeable difference.
> > > >
> > > > I'm happy to hear from sceptical performance checkers :o)
> > >
> > > On a reasonably fast amd64 box, this increases GENERIC.MP make -j6 build
> > > time from
> > > ~3m06s to ~3m44s, which seems a bit much to me.
> >
> > Do you know if this is due to an increase of %spin time?
> >
> > > Replacing smr_barrier() with smr_flush() reduces the overhead to a couple
> > > of
> > > seconds, and it seems warranted here.
> >
> > Could you try the diff below that only call smr_barrier() for multi-
> > threaded processes with threads still in the list. I guess this also
> > answers guenther@'s question. The same could be done with smr_flush().
>
> I'm wondering if smr_grace_wait() could be improved on amd64, assuming
> SMT is disabled, by skipping offline CPUs.
This doesn't make much of a difference when using smr_barrier(), but with
smr_flush() it removes much of the overhead on this system with 8 of 16 cpus
online. Of course as Visa and Mark point out this is risky without more
guarantees about what offline cpus are actually doing. If we start using SMR
in ways that make the delay visible to user processes, it'll probably be worth
looking at.
>
> Index: kern/kern_smr.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_smr.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 kern_smr.c
> --- kern/kern_smr.c 3 Apr 2020 03:36:56 -0000 1.8
> +++ kern/kern_smr.c 2 Dec 2020 18:41:29 -0000
> @@ -142,7 +142,7 @@ smr_grace_wait(void)
>
> ci_start = curcpu();
> CPU_INFO_FOREACH(cii, ci) {
> - if (ci == ci_start)
> + if (ci == ci_start || !cpu_is_online(ci))
> continue;
> sched_peg_curproc(ci);
> }
>
