[PATCH] Cygwin: Implement CPU_SET(3) macros

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Cygwin: Implement CPU_SET(3) macros

Mark Geisert
This patch supplies an implementation of the CPU_SET(3) processor
affinity macros as documented on the relevant Linux man page.
---
 winsup/cygwin/include/sys/cpuset.h | 62 +++++++++++++++++++++++++++---
 winsup/cygwin/sched.cc             |  8 ++--
 2 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/include/sys/cpuset.h b/winsup/cygwin/include/sys/cpuset.h
index 4857b879d..9c8417b73 100644
--- a/winsup/cygwin/include/sys/cpuset.h
+++ b/winsup/cygwin/include/sys/cpuset.h
@@ -14,20 +14,70 @@ extern "C" {
 #endif
 
 typedef __SIZE_TYPE__ __cpu_mask;
-#define __CPU_SETSIZE 1024  // maximum number of logical processors tracked
-#define __NCPUBITS (8 * sizeof (__cpu_mask))  // max size of processor group
-#define __CPU_GROUPMAX (__CPU_SETSIZE / __NCPUBITS)  // maximum group number
+#define CPU_SETSIZE  1024  // maximum number of logical processors tracked
+#define NCPUBITS     (8 * sizeof (__cpu_mask))  // max size of processor group
+#define CPU_GROUPMAX (CPU_SETSIZE / NCPUBITS)  // maximum group number
 
-#define __CPUELT(cpu)   ((cpu) / __NCPUBITS)
-#define __CPUMASK(cpu)  ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
+#define CPU_WORD(cpu) ((cpu) / NCPUBITS)
+#define CPU_MASK(cpu) ((__cpu_mask) 1 << ((cpu) % NCPUBITS))
 
 typedef struct
 {
-  __cpu_mask __bits[__CPU_GROUPMAX];
+  __cpu_mask __bits[CPU_GROUPMAX];
 } cpu_set_t;
 
 int __sched_getaffinity_sys (pid_t, size_t, cpu_set_t *);
 
+/* These macros alloc or free dynamically-sized cpu sets of size 'num' cpus.
+   Allocations are padded such that full-word operations can be done easily. */
+#define CPU_ALLOC_SIZE(num) ((num+NCPUBITS-1) / NCPUBITS) * sizeof (__cpu_mask)
+#define CPU_ALLOC(num)    __builtin_malloc (CPU_ALLOC_SIZE(num))
+#define CPU_FREE(set)    __builtin_free (set)
+
+/* These _S macros operate on dynamically-sized cpu sets of size 'siz' bytes */
+#define CPU_ZERO_S(siz, set) __builtin_memset (set, 0, siz)
+
+#define CPU_SET_S(cpu,siz,set) (set)->__bits[CPU_WORD(cpu)] |= CPU_MASK(cpu)
+#define CPU_CLR_S(cpu,siz,set) (set)->__bits[CPU_WORD(cpu)] &= ~(CPU_MASK(cpu))
+#define CPU_ISSET_S(cpu,siz,set) (set)->__bits[CPU_WORD(cpu)] & CPU_MASK(cpu)
+
+#define CPU_COUNT_S(siz, set) \
+      ({int tot = 0;\
+ for (int i = 0; i < siz / sizeof (__cpu_mask); i++) \
+  tot += __builtin_popcountl ((set)->__bits[i]); \
+ tot;})
+
+#define CPU_AND_S(siz, dst, src1, src2) \
+ for (int i = 0; i < siz / sizeof (__cpu_mask); i++) \
+  (dst)->__bits[i] = (src1)->__bits[i] & (src2)->__bits[i];
+
+#define CPU_OR_S(siz, dst, src1, src2) \
+ for (int i = 0; i < siz / sizeof (__cpu_mask); i++) \
+  (dst)->__bits[i] = (src1)->__bits[i] | (src2)->__bits[i];
+
+#define CPU_XOR_S(siz, dst, src1, src2) \
+ for (int i = 0; i < siz / sizeof (__cpu_mask); i++) \
+  (dst)->__bits[i] = (src1)->__bits[i] ^ (src2)->__bits[i];
+
+#define CPU_EQUAL_S(siz, src1, src2) \
+      ({int res = 1; \
+ for (int i = 0; res && i < siz / sizeof (__cpu_mask); i++) \
+  res &= (src1)->__bits[i] == (src2)->__bits[i]; \
+ res;})
+
+/* These macros operate on fixed-size cpu sets of size CPU_SETSIZE cpus */
+#define CPU_ZERO(set)  CPU_ZERO_S(sizeof (cpu_set_t), set)
+
+#define CPU_SET(cpu, set)  CPU_SET_S(cpu, sizeof (cpu_set_t), set)
+#define CPU_CLR(cpu, set)  CPU_CLR_S(cpu, sizeof (cpu_set_t), set)
+#define CPU_ISSET(cpu, set)  CPU_ISSET_S(cpu, sizeof (cpu_set_t), set)
+
+#define CPU_COUNT(set)  CPU_COUNT_S(sizeof (cpu_set_t), set)
+#define CPU_AND(dst, src1, src2)  CPU_AND_S(sizeof (cpu_set_t), dst, src1, src2)
+#define CPU_OR(dst, src1, src2)   CPU_OR_S(sizeof (cpu_set_t), dst, src1, src2)
+#define CPU_XOR(dst, src1, src2)  CPU_XOR_S(sizeof (cpu_set_t), dst, src1, src2)
+#define CPU_EQUAL(src1, src2)  CPU_EQUAL_S(sizeof (cpu_set_t), src1, src2)
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/winsup/cygwin/sched.cc b/winsup/cygwin/sched.cc
index fdb8ba738..73c861df9 100644
--- a/winsup/cygwin/sched.cc
+++ b/winsup/cygwin/sched.cc
@@ -578,8 +578,8 @@ __sched_getaffinity_sys (pid_t pid, size_t sizeof_set, cpu_set_t *set)
       memset (set, 0, sizeof_set);
       if (wincap.has_processor_groups () && __get_group_count () > 1)
         {
-          USHORT groupcount = __CPU_GROUPMAX;
-          USHORT grouparray[__CPU_GROUPMAX];
+          USHORT groupcount = CPU_GROUPMAX;
+          USHORT grouparray[CPU_GROUPMAX];
 
           if (!GetProcessGroupAffinity (process, &groupcount, grouparray))
             {
@@ -683,8 +683,8 @@ sched_setaffinity (pid_t pid, size_t sizeof_set, const cpu_set_t *set)
      p->dwProcessId) : GetCurrentProcess ();
       if (wincap.has_processor_groups () && __get_group_count () > 1)
  {
-  USHORT groupcount = __CPU_GROUPMAX;
-  USHORT grouparray[__CPU_GROUPMAX];
+  USHORT groupcount = CPU_GROUPMAX;
+  USHORT grouparray[CPU_GROUPMAX];
 
   if (!GetProcessGroupAffinity (process, &groupcount, grouparray))
     {
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: Implement CPU_SET(3) macros

Corinna Vinschen-2
Hi Mark,

On Jun 30 15:59, Mark Geisert wrote:

> This patch supplies an implementation of the CPU_SET(3) processor
> affinity macros as documented on the relevant Linux man page.
> ---
>  winsup/cygwin/include/sys/cpuset.h | 62 +++++++++++++++++++++++++++---
>  winsup/cygwin/sched.cc             |  8 ++--
>  2 files changed, 60 insertions(+), 10 deletions(-)
>
> diff --git a/winsup/cygwin/include/sys/cpuset.h b/winsup/cygwin/include/sys/cpuset.h
> index 4857b879d..9c8417b73 100644
> --- a/winsup/cygwin/include/sys/cpuset.h
> +++ b/winsup/cygwin/include/sys/cpuset.h
> @@ -14,20 +14,70 @@ extern "C" {
>  #endif
>  
>  typedef __SIZE_TYPE__ __cpu_mask;
> -#define __CPU_SETSIZE 1024  // maximum number of logical processors tracked
> -#define __NCPUBITS (8 * sizeof (__cpu_mask))  // max size of processor group
> -#define __CPU_GROUPMAX (__CPU_SETSIZE / __NCPUBITS)  // maximum group number
> +#define CPU_SETSIZE  1024  // maximum number of logical processors tracked
> +#define NCPUBITS     (8 * sizeof (__cpu_mask))  // max size of processor group
> +#define CPU_GROUPMAX (CPU_SETSIZE / NCPUBITS)  // maximum group number
>  
> -#define __CPUELT(cpu)   ((cpu) / __NCPUBITS)
> -#define __CPUMASK(cpu)  ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
> +#define CPU_WORD(cpu) ((cpu) / NCPUBITS)
> +#define CPU_MASK(cpu) ((__cpu_mask) 1 << ((cpu) % NCPUBITS))
I wouldn't do that.  Three problems:

- The non-underscored definitions should only be exposed #if __GNU_VISIBLE
  because otherwise they clutter the application namespace.

- CPU_WORD and CPU_MASK don't exist at all in glibc, so I don't see
  why you rename __CPUELT and __CPUMASK at all.

- CPU_GROUPMAX does not exist in glibc either. As a non-standard
  macro it should be kept underscored.

Keep (and use) the underscored variations throughout, and only expose
the non-underscored macro set #if __GNU_VISIBLE.

There's also the request from Sebastian on the newlib list to
consolidate the cpuset stuff from RTEMS and Cygwin into a single
definition.  It's not exactly straightforward given the different
definition of cpuset_t from FreeBSD, but it's probably not very
complicated either.  Maybe something for after your vaca?


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: Implement CPU_SET(3) macros

Mark Geisert
Corinna Vinschen wrote:

> Hi Mark,
>
> On Jun 30 15:59, Mark Geisert wrote:
>> This patch supplies an implementation of the CPU_SET(3) processor
>> affinity macros as documented on the relevant Linux man page.
>> ---
>>   winsup/cygwin/include/sys/cpuset.h | 62 +++++++++++++++++++++++++++---
>>   winsup/cygwin/sched.cc             |  8 ++--
>>   2 files changed, 60 insertions(+), 10 deletions(-)
>>
>> diff --git a/winsup/cygwin/include/sys/cpuset.h b/winsup/cygwin/include/sys/cpuset.h
>> index 4857b879d..9c8417b73 100644
>> --- a/winsup/cygwin/include/sys/cpuset.h
>> +++ b/winsup/cygwin/include/sys/cpuset.h
>> @@ -14,20 +14,70 @@ extern "C" {
>>   #endif
>>  
>>   typedef __SIZE_TYPE__ __cpu_mask;
>> -#define __CPU_SETSIZE 1024  // maximum number of logical processors tracked
>> -#define __NCPUBITS (8 * sizeof (__cpu_mask))  // max size of processor group
>> -#define __CPU_GROUPMAX (__CPU_SETSIZE / __NCPUBITS)  // maximum group number
>> +#define CPU_SETSIZE  1024  // maximum number of logical processors tracked
>> +#define NCPUBITS     (8 * sizeof (__cpu_mask))  // max size of processor group
>> +#define CPU_GROUPMAX (CPU_SETSIZE / NCPUBITS)  // maximum group number
>>  
>> -#define __CPUELT(cpu)   ((cpu) / __NCPUBITS)
>> -#define __CPUMASK(cpu)  ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
>> +#define CPU_WORD(cpu) ((cpu) / NCPUBITS)
>> +#define CPU_MASK(cpu) ((__cpu_mask) 1 << ((cpu) % NCPUBITS))
>
> I wouldn't do that.  Three problems:
>
> - The non-underscored definitions should only be exposed #if __GNU_VISIBLE
>    because otherwise they clutter the application namespace.

Ah, I didn't consider that.  Will wrap appropriately.

> - CPU_WORD and CPU_MASK don't exist at all in glibc, so I don't see
>    why you rename __CPUELT and __CPUMASK at all.

Conflicting goals and I made a wrong choice.  I thought leading underscores
meant "implementation details" and so should not be directly used by apps.  And
I was trying to distinguish this implementation from glibc's per Eric's comment
on the newlib list.  I figured these two macros would be useful to app coders so
renamed them to indicate they can use them.

And then I found taskset uses one or more of the underscored ones... So I'll
revert this bit.

> - CPU_GROUPMAX does not exist in glibc either. As a non-standard
>    macro it should be kept underscored.

Not in glibc because it's a Windows-related construct.  Will keep underscored.

> Keep (and use) the underscored variations throughout, and only expose
> the non-underscored macro set #if __GNU_VISIBLE.

Got it.  Thanks!

> There's also the request from Sebastian on the newlib list to
> consolidate the cpuset stuff from RTEMS and Cygwin into a single
> definition.  It's not exactly straightforward given the different
> definition of cpuset_t from FreeBSD, but it's probably not very
> complicated either.  Maybe something for after your vaca?

Yes, later if at all :-).  I looked at the FreeBSD 11.0 man page for CPUSET(9),
as they call it, from
https://www.freebsd.org/cgi/man.cgi?query=cpuset&sektion=9&apropos=0&manpath=FreeBSD+11.0-RELEASE+and+Ports
and see that there's some family resemblance but many differences.  I'll have to
look at RTEMS' again to see how they combined FreeBSD and Linux variants.

I've also found that taskset isn't working properly on my build system with the
new CPU_SET code, though my other testcases are.  So even as submitted, and
fixed per your comments here, there's a bit more to be done.

..mark
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: Implement CPU_SET(3) macros

Corinna Vinschen-2
Hi Mark,

On Jul  1 01:55, Mark Geisert wrote:

> Corinna Vinschen wrote:
> > On Jun 30 15:59, Mark Geisert wrote:
> > > This patch supplies an implementation of the CPU_SET(3) processor
> > > affinity macros as documented on the relevant Linux man page.
> > > ---
> > >   winsup/cygwin/include/sys/cpuset.h | 62 +++++++++++++++++++++++++++---
> > >   winsup/cygwin/sched.cc             |  8 ++--
> > >   2 files changed, 60 insertions(+), 10 deletions(-)
> > > [...]
> > > +#define CPU_SETSIZE  1024  // maximum number of logical processors tracked
> > > +#define NCPUBITS     (8 * sizeof (__cpu_mask))  // max size of processor group
> > > +#define CPU_GROUPMAX (CPU_SETSIZE / NCPUBITS)  // maximum group number
> > > -#define __CPUELT(cpu)   ((cpu) / __NCPUBITS)
> > > -#define __CPUMASK(cpu)  ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
> > > +#define CPU_WORD(cpu) ((cpu) / NCPUBITS)
> > > +#define CPU_MASK(cpu) ((__cpu_mask) 1 << ((cpu) % NCPUBITS))
> >
> > I wouldn't do that.  Three problems:
> > [...]
> > There's also the request from Sebastian on the newlib list to
> > consolidate the cpuset stuff from RTEMS and Cygwin into a single
> > definition.
> > [...]
> I've also found that taskset isn't working properly on my build system with
> the new CPU_SET code, though my other testcases are.  So even as submitted,
> and fixed per your comments here, there's a bit more to be done.
>
> ..mark
any chance to pick this up again?


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: Implement CPU_SET(3) macros

Mark Geisert
Corinna Vinschen wrote:

> Hi Mark,
>
> On Jul  1 01:55, Mark Geisert wrote:
>> Corinna Vinschen wrote:
>>> On Jun 30 15:59, Mark Geisert wrote:
>>>> This patch supplies an implementation of the CPU_SET(3) processor
>>>> affinity macros as documented on the relevant Linux man page.
>>>> ---
>>>>    winsup/cygwin/include/sys/cpuset.h | 62 +++++++++++++++++++++++++++---
>>>>    winsup/cygwin/sched.cc             |  8 ++--
>>>>    2 files changed, 60 insertions(+), 10 deletions(-)
>>>> [...]
>>>> +#define CPU_SETSIZE  1024  // maximum number of logical processors tracked
>>>> +#define NCPUBITS     (8 * sizeof (__cpu_mask))  // max size of processor group
>>>> +#define CPU_GROUPMAX (CPU_SETSIZE / NCPUBITS)  // maximum group number
>>>> -#define __CPUELT(cpu)   ((cpu) / __NCPUBITS)
>>>> -#define __CPUMASK(cpu)  ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
>>>> +#define CPU_WORD(cpu) ((cpu) / NCPUBITS)
>>>> +#define CPU_MASK(cpu) ((__cpu_mask) 1 << ((cpu) % NCPUBITS))
>>>
>>> I wouldn't do that.  Three problems:
>>> [...]
>>> There's also the request from Sebastian on the newlib list to
>>> consolidate the cpuset stuff from RTEMS and Cygwin into a single
>>> definition.
>>> [...]
>> I've also found that taskset isn't working properly on my build system with
>> the new CPU_SET code, though my other testcases are.  So even as submitted,
>> and fixed per your comments here, there's a bit more to be done.
>>
>> ..mark
>
> any chance to pick this up again?

Hi; yes, certainly. I'm back but ill. It may be a week or so before I have an
update/fix. Top of my list of pending items that require concentration ;-).

..mark
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: Implement CPU_SET(3) macros

Corinna Vinschen-2
On Jul 25 14:15, Mark Geisert wrote:

> Corinna Vinschen wrote:
> > Hi Mark,
> >
> > On Jul  1 01:55, Mark Geisert wrote:
> > > Corinna Vinschen wrote:
> > > > On Jun 30 15:59, Mark Geisert wrote:
> > > > > This patch supplies an implementation of the CPU_SET(3) processor
> > > > > affinity macros as documented on the relevant Linux man page.
> > > > > ---
> > > > >    winsup/cygwin/include/sys/cpuset.h | 62 +++++++++++++++++++++++++++---
> > > > >    winsup/cygwin/sched.cc             |  8 ++--
> > > > >    2 files changed, 60 insertions(+), 10 deletions(-)
> > > > > [...]
> > > > > +#define CPU_SETSIZE  1024  // maximum number of logical processors tracked
> > > > > +#define NCPUBITS     (8 * sizeof (__cpu_mask))  // max size of processor group
> > > > > +#define CPU_GROUPMAX (CPU_SETSIZE / NCPUBITS)  // maximum group number
> > > > > -#define __CPUELT(cpu)   ((cpu) / __NCPUBITS)
> > > > > -#define __CPUMASK(cpu)  ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
> > > > > +#define CPU_WORD(cpu) ((cpu) / NCPUBITS)
> > > > > +#define CPU_MASK(cpu) ((__cpu_mask) 1 << ((cpu) % NCPUBITS))
> > > >
> > > > I wouldn't do that.  Three problems:
> > > > [...]
> > > > There's also the request from Sebastian on the newlib list to
> > > > consolidate the cpuset stuff from RTEMS and Cygwin into a single
> > > > definition.
> > > > [...]
> > > I've also found that taskset isn't working properly on my build system with
> > > the new CPU_SET code, though my other testcases are.  So even as submitted,
> > > and fixed per your comments here, there's a bit more to be done.
> > >
> > > ..mark
> >
> > any chance to pick this up again?
>
> Hi; yes, certainly. I'm back but ill.
I hope it's nothing serious.  Please make sure you're getting well
before diving too deep into code again :)

> It may be a week or so before I have
> an update/fix. Top of my list of pending items that require concentration
> ;-).
>
> ..mark

Sounds good to me.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: Implement CPU_SET(3) macros

Mark Geisert
On Fri, 26 Jul 2019, Corinna Vinschen wrote:

> On Jul 25 14:15, Mark Geisert wrote:
>> Corinna Vinschen wrote:
>>> Hi Mark,
>>>
>>> On Jul  1 01:55, Mark Geisert wrote:
>>>> Corinna Vinschen wrote:
>>>>> On Jun 30 15:59, Mark Geisert wrote:
>>>>>> This patch supplies an implementation of the CPU_SET(3) processor
>>>>>> affinity macros as documented on the relevant Linux man page.
>>>>>> ---
>>>>>>    winsup/cygwin/include/sys/cpuset.h | 62 +++++++++++++++++++++++++++---
>>>>>>    winsup/cygwin/sched.cc             |  8 ++--
>>>>>>    2 files changed, 60 insertions(+), 10 deletions(-)
>>>>>> [...]
>>>>>> +#define CPU_SETSIZE  1024  // maximum number of logical processors tracked
>>>>>> +#define NCPUBITS     (8 * sizeof (__cpu_mask))  // max size of processor group
>>>>>> +#define CPU_GROUPMAX (CPU_SETSIZE / NCPUBITS)  // maximum group number
>>>>>> -#define __CPUELT(cpu)   ((cpu) / __NCPUBITS)
>>>>>> -#define __CPUMASK(cpu)  ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
>>>>>> +#define CPU_WORD(cpu) ((cpu) / NCPUBITS)
>>>>>> +#define CPU_MASK(cpu) ((__cpu_mask) 1 << ((cpu) % NCPUBITS))
>>>>>
>>>>> I wouldn't do that.  Three problems:
>>>>> [...]
>>>>> There's also the request from Sebastian on the newlib list to
>>>>> consolidate the cpuset stuff from RTEMS and Cygwin into a single
>>>>> definition.
>>>>> [...]
>>>> I've also found that taskset isn't working properly on my build system with
>>>> the new CPU_SET code, though my other testcases are.  So even as submitted,
>>>> and fixed per your comments here, there's a bit more to be done.
>>>>
>>>> ..mark
>>>
>>> any chance to pick this up again?

I've been looking at this suggestion to consolidate the cpuset stuff from
RTEMS and Cygwin.  There is no location common to both platforms to put
this stuff other than Newlib's libc directory or maybe a non-sys subdir
of libc.  If situated there, it could impact other newlib platforms,
possibly.  It also seems a little messy to me to have to put four include
files there.. cpuset.h, _cpuset.h, bitset.h, and _bitset.h.  Maybe I'm
overthinking it.

RTEMS' cpuset.h is built on bitset.h, which is fine but the cpuset.h I
wrote is self-contained and makes use of gcc builtins rather than C
library calls for malloc, free, popcount, etc.  Mine is 32/64 agnostic, I
believe RTEMS is too but I'm not totally sure; it depends on the length
of 'long' items.

RTEMS' implementation includes some code modules needing to be linked into
libc while the one I wrote is all in header files with some inline code.

These are all just minor implementation differences but I'm still hung up
on the question of just where a common implementation could be placed in
the source tree so both RTEMS and Cygwin can use them but other newlib
platforms won't be tripped up.

..mark

P.S. IRC would be better for this kind of discussion. I'm waiting for my
freenode registration to be processed.....
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: Implement CPU_SET(3) macros

Corinna Vinschen-2
On Jul 30 02:25, Mark Geisert wrote:

> On Fri, 26 Jul 2019, Corinna Vinschen wrote:
> > On Jul 25 14:15, Mark Geisert wrote:
> > > Corinna Vinschen wrote:
> > > > Hi Mark,
> > > >
> > > > On Jul  1 01:55, Mark Geisert wrote:
> > > > > Corinna Vinschen wrote:
> > > > > > On Jun 30 15:59, Mark Geisert wrote:
> > > > > > > This patch supplies an implementation of the CPU_SET(3) processor
> > > > > > > affinity macros as documented on the relevant Linux man page.
> > > > > > > ---
> > > > > > >    winsup/cygwin/include/sys/cpuset.h | 62 +++++++++++++++++++++++++++---
> > > > > > >    winsup/cygwin/sched.cc             |  8 ++--
> > > > > > >    2 files changed, 60 insertions(+), 10 deletions(-)
> > > > > > > [...]
> > > > > > > +#define CPU_SETSIZE  1024  // maximum number of logical processors tracked
> > > > > > > +#define NCPUBITS     (8 * sizeof (__cpu_mask))  // max size of processor group
> > > > > > > +#define CPU_GROUPMAX (CPU_SETSIZE / NCPUBITS)  // maximum group number
> > > > > > > -#define __CPUELT(cpu)   ((cpu) / __NCPUBITS)
> > > > > > > -#define __CPUMASK(cpu)  ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
> > > > > > > +#define CPU_WORD(cpu) ((cpu) / NCPUBITS)
> > > > > > > +#define CPU_MASK(cpu) ((__cpu_mask) 1 << ((cpu) % NCPUBITS))
> > > > > >
> > > > > > I wouldn't do that.  Three problems:
> > > > > > [...]
> > > > > > There's also the request from Sebastian on the newlib list to
> > > > > > consolidate the cpuset stuff from RTEMS and Cygwin into a single
> > > > > > definition.
> > > > > > [...]
> > > > > I've also found that taskset isn't working properly on my build system with
> > > > > the new CPU_SET code, though my other testcases are.  So even as submitted,
> > > > > and fixed per your comments here, there's a bit more to be done.
> > > > >
> > > > > ..mark
> > > >
> > > > any chance to pick this up again?
>
> I've been looking at this suggestion to consolidate the cpuset stuff from
> RTEMS and Cygwin.  There is no location common to both platforms to put this
> stuff other than Newlib's libc directory or maybe a non-sys subdir of libc.
> If situated there, it could impact other newlib platforms, possibly.  It
> also seems a little messy to me to have to put four include files there..
> cpuset.h, _cpuset.h, bitset.h, and _bitset.h.  Maybe I'm overthinking it.
Well, the files are already there, just in the rtems subdir instead of the
generic one.  They just have to be moved, and the _cpuset.h file needs
a duplicate in cygwin's include dir to account for the Cygwin-specific parts.
It's not that you generate a lot of new files.

> RTEMS' cpuset.h is built on bitset.h, which is fine but the cpuset.h I wrote
> is self-contained and makes use of gcc builtins rather than C library calls
> for malloc, free, popcount, etc.  Mine is 32/64 agnostic, I believe RTEMS is
> too but I'm not totally sure; it depends on the length of 'long' items.
>
> RTEMS' implementation includes some code modules needing to be linked into
> libc while the one I wrote is all in header files with some inline code.
>
> These are all just minor implementation differences but I'm still hung up on
> the question of just where a common implementation could be placed in the
> source tree so both RTEMS and Cygwin can use them but other newlib platforms
> won't be tripped up.
They won't.  The rtems/FreeBSD implementation is CPU agnostic so the stuff
could go into the generic tree.  If your platform doesn't implement it,
just don't include it in your target code.

However, it sounds like you're pretty uncomfortable with the rtems/FreeBSD
implementation.  If so, combining them is not required.  It would just
be a nice-to-have since it also leads to more people interested in keeping
it in a good shape.

> ..mark
>
> P.S. IRC would be better for this kind of discussion. I'm waiting for my
> freenode registration to be processed.....

Great!


Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH v2] Cygwin: Implement CPU_SET(3) macros

Mark Geisert
This patch supplies an implementation of the CPU_SET(3) processor
affinity macros as documented on the relevant Linux man page.

There is a different implementation of cpusets under libc/sys/RTEMS that
has FreeBSD compatibility and is built on top of FreeBSD bitsets.  This
implementation can be combined with that one if necessary in the future.

---
 winsup/cygwin/include/sys/cpuset.h | 64 +++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/include/sys/cpuset.h b/winsup/cygwin/include/sys/cpuset.h
index 4857b879d..2056f6af7 100644
--- a/winsup/cygwin/include/sys/cpuset.h
+++ b/winsup/cygwin/include/sys/cpuset.h
@@ -18,8 +18,8 @@ typedef __SIZE_TYPE__ __cpu_mask;
 #define __NCPUBITS (8 * sizeof (__cpu_mask))  // max size of processor group
 #define __CPU_GROUPMAX (__CPU_SETSIZE / __NCPUBITS)  // maximum group number
 
-#define __CPUELT(cpu)   ((cpu) / __NCPUBITS)
-#define __CPUMASK(cpu)  ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
+#define __CPUELT(cpu)  ((cpu) / __NCPUBITS)
+#define __CPUMASK(cpu) ((__cpu_mask) 1 << ((cpu) % __NCPUBITS))
 
 typedef struct
 {
@@ -28,6 +28,66 @@ typedef struct
 
 int __sched_getaffinity_sys (pid_t, size_t, cpu_set_t *);
 
+/* These macros alloc or free dynamically-sized cpu sets of size 'num' cpus.
+   Allocations are padded such that full-word operations can be done easily. */
+#define CPU_ALLOC_SIZE(num) ((num+__NCPUBITS-1) / __NCPUBITS) * sizeof (__cpu_mask)
+#define CPU_ALLOC(num)      __builtin_malloc (CPU_ALLOC_SIZE(num))
+#define CPU_FREE(set)       __builtin_free (set)
+
+/* These _S macros operate on dynamically-sized cpu sets of size 'siz' bytes */
+#define CPU_ZERO_S(siz, set)    __builtin_memset (set, 0, siz)
+
+#define CPU_SET_S(cpu,siz,set) \
+ if (cpu < 8 * siz) \
+  (set)->__bits[__CPUELT(cpu)] |= __CPUMASK(cpu);
+
+#define CPU_CLR_S(cpu,siz,set) \
+ if (cpu < 8 * siz) \
+  (set)->__bits[__CPUELT(cpu)] &= ~(__CPUMASK(cpu));
+
+#define CPU_ISSET_S(cpu,siz,set) \
+      ({int res = 0; \
+ if (cpu < 8 * siz) \
+  res = !!((set)->__bits[__CPUELT(cpu)] & __CPUMASK(cpu)); \
+ res;})
+
+#define CPU_COUNT_S(siz, set) \
+      ({int tot = 0;\
+ for (int i = 0; i < siz / sizeof (__cpu_mask); i++) \
+  tot += __builtin_popcountl ((set)->__bits[i]); \
+ tot;})
+
+#define CPU_AND_S(siz, dst, src1, src2) \
+ for (int i = 0; i < siz / sizeof (__cpu_mask); i++) \
+  (dst)->__bits[i] = (src1)->__bits[i] & (src2)->__bits[i];
+
+#define CPU_OR_S(siz, dst, src1, src2) \
+ for (int i = 0; i < siz / sizeof (__cpu_mask); i++) \
+  (dst)->__bits[i] = (src1)->__bits[i] | (src2)->__bits[i];
+
+#define CPU_XOR_S(siz, dst, src1, src2) \
+ for (int i = 0; i < siz / sizeof (__cpu_mask); i++) \
+  (dst)->__bits[i] = (src1)->__bits[i] ^ (src2)->__bits[i];
+
+#define CPU_EQUAL_S(siz, src1, src2) \
+      ({int res = 1; \
+ for (int i = 0; res && i < siz / sizeof (__cpu_mask); i++) \
+  res &= (src1)->__bits[i] == (src2)->__bits[i]; \
+ res;})
+
+/* These macros operate on fixed-size cpu sets of size __CPU_SETSIZE cpus */
+#define CPU_ZERO(set)             CPU_ZERO_S(sizeof (cpu_set_t), set)
+
+#define CPU_SET(cpu, set)         CPU_SET_S(cpu, sizeof (cpu_set_t), set)
+#define CPU_CLR(cpu, set)         CPU_CLR_S(cpu, sizeof (cpu_set_t), set)
+#define CPU_ISSET(cpu, set)       CPU_ISSET_S(cpu, sizeof (cpu_set_t), set)
+
+#define CPU_COUNT(set)            CPU_COUNT_S(sizeof (cpu_set_t), set)
+#define CPU_AND(dst, src1, src2)  CPU_AND_S(sizeof (cpu_set_t), dst, src1, src2)
+#define CPU_OR(dst, src1, src2)   CPU_OR_S(sizeof (cpu_set_t), dst, src1, src2)
+#define CPU_XOR(dst, src1, src2)  CPU_XOR_S(sizeof (cpu_set_t), dst, src1, src2)
+#define CPU_EQUAL(src1, src2)     CPU_EQUAL_S(sizeof (cpu_set_t), src1, src2)
+
 #ifdef __cplusplus
 }
 #endif
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] Cygwin: Implement CPU_SET(3) macros

Mark Geisert
Sorry for the repetition...

On Sun, 4 Aug 2019, Mark Geisert wrote:
> This patch supplies an implementation of the CPU_SET(3) processor
> affinity macros as documented on the relevant Linux man page.
>
> There is a different implementation of cpusets under libc/sys/RTEMS that
> has FreeBSD compatibility and is built on top of FreeBSD bitsets.  This
> implementation can be combined with that one if necessary in the future.

If the 2nd para is to be kept, please adjust it to read as follows:

There is a mostly superset implementation of cpusets under newlib's
libc/sys/RTEMS/include/sys that has Linux and FreeBSD compatibility
and is built on top of FreeBSD bitsets.  This Cygwin implementation
and the RTEMS one could be combined if desired at some future point.

Thanks,

..mark
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] Cygwin: Implement CPU_SET(3) macros

Corinna Vinschen-2
In reply to this post by Mark Geisert
On Aug  4 15:45, Mark Geisert wrote:

> This patch supplies an implementation of the CPU_SET(3) processor
> affinity macros as documented on the relevant Linux man page.
>
> There is a different implementation of cpusets under libc/sys/RTEMS that
> has FreeBSD compatibility and is built on top of FreeBSD bitsets.  This
> implementation can be combined with that one if necessary in the future.
>
> ---
>  winsup/cygwin/include/sys/cpuset.h | 64 +++++++++++++++++++++++++++++-
>  1 file changed, 62 insertions(+), 2 deletions(-)
Pushed with the commit message tweak and additional doc changes.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment