Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 30 Mar 2002 00:19:29 -0500
From:      Jake Burkholder <jake@locore.ca>
To:        Matthew Dillon <dillon@apollo.backplane.com>
Cc:        John Baldwin <jhb@FreeBSD.ORG>, freebsd-smp@FreeBSD.ORG
Subject:   Re: RE: Syscall contention tests return, userret() bugs/issues.
Message-ID:  <20020330001929.H40695@locore.ca>
In-Reply-To: <200203292305.g2TN5nX75021@apollo.backplane.com>; from dillon@apollo.backplane.com on Fri, Mar 29, 2002 at 03:05:49PM -0800
References:  <XFMail.20020329171752.jhb@FreeBSD.org> <200203292305.g2TN5nX75021@apollo.backplane.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Apparently, On Fri, Mar 29, 2002 at 03:05:49PM -0800,
	Matthew Dillon said words to the effect of;

> :Hmm, I'm not sure of the best way of handling this stat then.  ++*PCPU_PTR()
> :might actually be ok at least for all of our current archs.  (If you migrate
> :the update the counter of another CPU but that would be rare.)
> 
>     I think the easiest thing to do is to create a macro in the MI pcpu.h:
> 
> 	PCPU_LAZY_COUNTER(cnt.v_syscall, 1);
> 
>     Which would just do this for now:
> 
> 	*PCPU_PTR(arg) += count;
> 
>     With a big fat XXX comment describing the situation, allowing us to fix
>     it later.
> 
>     I was able to embed struct vmmeter into sys/pcpu.h trivially. 
>     sys/vmmeter.h is a low level include that does not depend on much
>     of anything, so it simply worked to do this:

I think its worth adding a PCPU_INC, which turns into an incl %fs:PC_FIELD
on i386.  alpha, ia64 and sparc64 at least use a resrved regitser for the
pcpu pointer which is declared as a register variable in C, so you can just
do pcpup->pc_field++.  To make it consistent you'd have to either disable
interrupts or use atomic ops, because none of these architectures have
add or increment instructions that work directly on memory afaik.  I'd
be inclined to just blow it off, but as you point out, this can lead to
serious inconsistencies.

> 
> Index: sys/pcpu.h
> ...
>  #ifdef _KERNEL
>  #include <sys/queue.h>
> +#include <sys/vmmeter.h>
>  #include <machine/pcpu.h>
> ... 
>  	PCPU_MD_FIELDS;
> +	struct vmmeter	pc_cnt;			/* VM stats counters */
>  };
>     
>     I'm going to let this thread sit over the weekend to allow other
>     people to chime in in regards to moving struct vmmeter into a per-cpu
>     area.  I think I've made a good case for it based on the cache
>     contention between cpu's.  If no major problems crop up by monday
>     I'll start with the syscall counter and then start moving all the
>     'cnt.*' counter code to use the per-cpu area instead.

This sounds ok to me.

> 
> :>  (matt)
> :>     This looks like a good area of work.  If nobody has code worked up
> :>     for it I can research the issue more, add the flag, and shift things
> :>     into ast().  (After I finish the stage-2 cpu_critical*() stuff which
> :>     re-inlines cpu_critical_enter() and cpu_critical_exit(), which I've
> :>     been talking to Jake about).
> :
> :   (john)
> :
> :Sure, Bruce already has some patches for this stuff so you might ask him about
> :it.  Jake might also have some ideas on the topic, but sounds good to me.
> :
> :-- 
> :
> :John Baldwin <jhb@FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
> 
>     I've sent an email to Bruce.

Yes, most of userret should be merged into ast().  Signals should trigger
an AST so you only do the sig = cursig() if there's really a signal there.
This has the advantage of streamlining the locking, as well getting the
128 bit arithmetic used for signals out of that path.  This may need some
tweaking, but I think most of the support for this is already there.  Ditto
for the reschedule check.  All that userret really needs to do is account
for profiling, and the PS_PROFIL flag doesn't need sched_lock if you're
just testing it, not setting or clearing.

One problem is clearing the ASTPENDING flag.  Currently this still needs
sched_lock in order to not distrub the other bits.  It would be better
to move it into a separate flags field in the kse, probably on its own.
I think that the flag should be cleared in the MD code, just after its
read as set and we still have interrupts disabled, just before ast() is
called.

Jake

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-smp" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020330001929.H40695>