getcpu after sched_setaffinity

getcpu after sched_setaffinity

[Ulrich Drepper]

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

The attached test program fails on a dual core (and probably SMP) 
machine on x86-64.  Depending on where the thread starts, in one of the 
iterations the sched_setffinity() call succeeds but then sched_getcpu() 
fails to report the correct CPU.

In set_cpus_allowed migrate_task() is called if the new CPU set does not 
include the current CPU.  I hope that migrate_task() also works for 
p==current.

This leaves the x86-64 vgetcpu() implementation as the weak point.  Is 
the caching causing problems?  Should migrate_task() make sure the cache 
is reset?

You need a very recent glibc to compile (glibc-2.5.90-22 in rawhide). 
If this is not available replace the sched_getcpu() call.  But make sure 
you pass a pointer to a cache.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

[-- Attachment #2: tst-getcpu.c --]
[-- Type: text/plain, Size: 875 bytes --]

#include <errno.h>
#include <stdio.h>
#include <sched.h>


int
main (void)
{
  cpu_set_t cs;
  if (sched_getaffinity (getpid (), sizeof (cs), &cs) != 0)
    {
      printf ("getaffinity failed: %m\n");
      return 1;
    }

  int result = 0;
  int cpu = 0;
  while (CPU_COUNT (&cs) != 0)
    {
      if (CPU_ISSET (cpu, &cs))
	{
	  cpu_set_t cs2;
	  CPU_ZERO (&cs2);
	  CPU_SET (cpu, &cs2);
	  if (sched_setaffinity (getpid (), sizeof (cs2), &cs2) != 0)
	    {
	      printf ("setaffinity(%d) failed: %m\n", cpu);
	      result = 1;
	    }
	  else
	    {
	      int cpu2 = sched_getcpu ();
	      if (cpu2 == -1 && errno == ENOSYS)
		{
		  puts ("getcpu syscall not implemented");
		  return 0;
		}
	      if (cpu2 != cpu)
		{
		  printf ("getcpu results %d not possible\n", cpu2);
		  result = 1;
		}
	    }
	  CPU_CLR (cpu, &cs);
	}
      ++cpu;
    }

  return result;
}

Re: getcpu after sched_setaffinity

[Andi Kleen]

On Thu, May 10, 2007 at 03:24:58PM -0700, Ulrich Drepper wrote:
> The attached test program fails on a dual core (and probably SMP) 
> machine on x86-64.  Depending on where the thread starts, in one of the 
> iterations the sched_setffinity() call succeeds but then sched_getcpu() 
> fails to report the correct CPU.
> 
> In set_cpus_allowed migrate_task() is called if the new CPU set does not 
> include the current CPU.  I hope that migrate_task() also works for 
> p==current.
> 
> This leaves the x86-64 vgetcpu() implementation as the weak point.  Is 
> the caching causing problems?  

Probably. In principle getcpu() (where does the sched_ come from btw?)
is only designed for the case where you don't set the affinity explicitely;
otherwise you should already know where you are and don't need it.

The cache is optimized for the case when you run without affinity
and change CPUs only rarely (which is normal) so it is kept valid for a 
jiffie. And you always need to handle an outdated result from getcpu
anyways because you can't disable preemption from user space and could
switch any time.

In short your test case has a broken design.

> is reset?

The vsyscall/kernel can't reset the cache because it is managed by the
application.

Hmm ok one could probably define memset(..., 0) as a invalidation
interface, but because of the considerations above i don't think
it is really needed.

-Andi

Re: getcpu after sched_setaffinity

[Ulrich Drepper]

Andi Kleen wrote:
> Probably. In principle getcpu() (where does the sched_ come from btw?)

getcpu() is an unacceptable name.  All te other functions dealing with 
CPU (sets, etc) have a sched_ prefix.


> is only designed for the case where you don't set the affinity explicitely;
> otherwise you should already know where you are and don't need it.

That's not true in general.  Yes, because I want to test vgetcpu() I 
restrict the set to just one CPU.

But if I have more than 2 "CPUs" and I set the affinity to two CPUs 
which currently are not used you cannot make this argument.

getcpu should always work correctly not only if you cannot determine it 
in another way.


> Hmm ok one could probably define memset(..., 0) as a invalidation
> interface, but because of the considerations above i don't think
> it is really needed.

It is needed.

For now I added the cache clearing in the setaffinity calls in libc. 
Resetting to cache to {0,0} seems to work.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

Re: getcpu after sched_setaffinity

[Andi Kleen]

On Thu, May 10, 2007 at 04:09:10PM -0700, Ulrich Drepper wrote:
> >is only designed for the case where you don't set the affinity explicitely;
> >otherwise you should already know where you are and don't need it.
> 
> That's not true in general.  Yes, because I want to test vgetcpu() I 
> restrict the set to just one CPU.
> 
> But if I have more than 2 "CPUs" and I set the affinity to two CPUs 
> which currently are not used you cannot make this argument.

All the users I talked to never mentioned this as a use case.
They were only interested in fully unrestricted programs. 

Anyways you can handle it too, just invalidate or do getcpu with 
a new cache afterwards.

Or just wait the jiffie for the cache to expire.

> 
> getcpu should always work correctly not only if you cannot determine it 
> in another way.

getcpu by design is approximate. If you don't understand this
don't bother using it.

-Andi

Re: getcpu after sched_setaffinity

[Bert Wesarg]

Hello,

(Please CC me, thank you)

Ulrich Drepper wrote:
> The attached test program fails on a dual core (and probably SMP) 
> machine on x86-64.  Depending on where the thread starts, in one of the 
> iterations the sched_setffinity() call succeeds but then sched_getcpu() 
> fails to report the correct CPU.
> 
> In set_cpus_allowed migrate_task() is called if the new CPU set does not 
> include the current CPU.  I hope that migrate_task() also works for 
> p==current.
> 
> This leaves the x86-64 vgetcpu() implementation as the weak point.  Is 
> the caching causing problems?  Should migrate_task() make sure the cache 
> is reset?
> 
> You need a very recent glibc to compile (glibc-2.5.90-22 in rawhide). 
> If this is not available replace the sched_getcpu() call.  But make sure 
> you pass a pointer to a cache.
> 
> 
A question not related to this problem, but may I ask why the libc don't
use[1] the linux/getcpu.h header to get the opaque struct vgetcpu_cache?

Thank You

Bert Wesarg

[1]: http://sourceware.org/ml/glibc-cvs/2007-q2/msg00098.html