[PATCH] add sched_getcpu

[PATCH] add sched_getcpu

[Nathan Zadoks]

This is a GNU extension, but a fairly minor one, for a system call that
otherwise has no libc wrapper.

Adding it was discussed previously, without any objections:
http://www.openwall.com/lists/musl/2015/05/08/24
---
 include/sched.h          | 3 +++
 src/sched/sched_getcpu.c | 8 ++++++++
 2 files changed, 11 insertions(+)
 create mode 100644 src/sched/sched_getcpu.c

diff --git a/include/sched.h b/include/sched.h
index 3e34a72..17f5e06 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -76,6 +76,9 @@ void free(void *);
 
 typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; } cpu_set_t;
 int __sched_cpucount(size_t, const cpu_set_t *);
+#ifdef _GNU_SOURCE
+int sched_getcpu(void);
+#endif
 int sched_getaffinity(pid_t, size_t, cpu_set_t *);
 int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
 
diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
new file mode 100644
index 0000000..a5f8ea7
--- /dev/null
+++ b/src/sched/sched_getcpu.c
@@ -0,0 +1,8 @@
+#define _GNU_SOURCE
+#include <sched.h>
+
+static int sched_getcpu(void) {
+  int c, s;
+  s = syscall(SYS_getcpu, &c, NULL, NULL);
+  return (s == 0) ? c : s;
+}
-- 
2.7.1

Re: [PATCH] add sched_getcpu

[Nathan Zadoks]

Whoops, that previous patch was bunk because I forgot to rerun git format-patch, sorry!

[PATCH] add sched_getcpu

[Nathan Zadoks]

This is a GNU extension, but a fairly minor one, for a system call that
otherwise has no libc wrapper.

Adding it was discussed previously, without any objections:
http://www.openwall.com/lists/musl/2015/05/08/24
---
 include/sched.h          |  3 +++
 src/sched/sched_getcpu.c | 10 ++++++++++
 2 files changed, 13 insertions(+)
 create mode 100644 src/sched/sched_getcpu.c

diff --git a/include/sched.h b/include/sched.h
index 3e34a72..17f5e06 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -76,6 +76,9 @@ void free(void *);
 
 typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; } cpu_set_t;
 int __sched_cpucount(size_t, const cpu_set_t *);
+#ifdef _GNU_SOURCE
+int sched_getcpu(void);
+#endif
 int sched_getaffinity(pid_t, size_t, cpu_set_t *);
 int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
 
diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
new file mode 100644
index 0000000..070d6e7
--- /dev/null
+++ b/src/sched/sched_getcpu.c
@@ -0,0 +1,10 @@
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <sched.h>
+#include "syscall.h"
+
+int sched_getcpu(void) {
+  int c, s;
+  s = syscall(SYS_getcpu, &c, NULL, NULL);
+  return (s == 0) ? c : s;
+}
-- 
2.7.1

[PATCH] add sched_getcpu

[Nathan Zadoks]

This is a GNU extension, but a fairly minor one, for a system call that
otherwise has no libc wrapper.

Adding it was discussed previously, without any objections:
http://www.openwall.com/lists/musl/2015/05/08/24
---
 include/sched.h          |  3 +++
 src/sched/sched_getcpu.c | 10 ++++++++++
 2 files changed, 13 insertions(+)
 create mode 100644 src/sched/sched_getcpu.c

diff --git a/include/sched.h b/include/sched.h
index 3e34a72..17f5e06 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -76,6 +76,9 @@ void free(void *);
 
 typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; } cpu_set_t;
 int __sched_cpucount(size_t, const cpu_set_t *);
+#ifdef _GNU_SOURCE
+int sched_getcpu(void);
+#endif
 int sched_getaffinity(pid_t, size_t, cpu_set_t *);
 int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
 
diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
new file mode 100644
index 0000000..070d6e7
--- /dev/null
+++ b/src/sched/sched_getcpu.c
@@ -0,0 +1,10 @@
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <sched.h>
+#include "syscall.h"
+
+int sched_getcpu(void) {
+  int c, s;
+  s = syscall(SYS_getcpu, &c, NULL, NULL);
+  return (s == 0) ? c : s;
+}
-- 
2.7.1

Re: [PATCH] add sched_getcpu

[Alexander Monakov]

On Mon, 29 Feb 2016, Nathan Zadoks wrote:

> This is a GNU extension, but a fairly minor one, for a system call that
> otherwise has no libc wrapper.
> 
> Adding it was discussed previously, without any objections:
> http://www.openwall.com/lists/musl/2015/05/08/24
> ---
>  include/sched.h          |  3 +++
>  src/sched/sched_getcpu.c | 10 ++++++++++
>  2 files changed, 13 insertions(+)
>  create mode 100644 src/sched/sched_getcpu.c
> 
> diff --git a/include/sched.h b/include/sched.h
> index 3e34a72..17f5e06 100644
> --- a/include/sched.h
> +++ b/include/sched.h
> @@ -76,6 +76,9 @@ void free(void *);
>  
>  typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; } cpu_set_t;
>  int __sched_cpucount(size_t, const cpu_set_t *);
> +#ifdef _GNU_SOURCE

This code is already under the same #ifdef.

> +int sched_getcpu(void);
> +#endif
>  int sched_getaffinity(pid_t, size_t, cpu_set_t *);
>  int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
>  
> diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
> new file mode 100644
> index 0000000..070d6e7
> --- /dev/null
> +++ b/src/sched/sched_getcpu.c
> @@ -0,0 +1,10 @@
> +#define _GNU_SOURCE
> +#include <stdlib.h>

Do you need this include?

> +#include <sched.h>

(this include could also be dropped; I think it's a matter of policy whether
such includes are desirable or not, so please wait for comment from Rich)

> +#include "syscall.h"
> +
> +int sched_getcpu(void) {
> +  int c, s;
> +  s = syscall(SYS_getcpu, &c, NULL, NULL);
> +  return (s == 0) ? c : s;

This is wrong, as it doesn't set errno on error, and does not produce -1. This
should be something like 'return s ? __syscall_ret(s) : c;' or maybe
'return __syscall_ret(s ? s : c);'.

Alexander

Re: [PATCH] add sched_getcpu

[Alexander Monakov]

There's also the question whether musl wants to use the VDSO here. According
to manpages, getcpu is provided in vdso as __vdso_getcpu on x86_64, x32, and
as __kernel_getcpu on ppc (on 64-bit kernels only, for both 32/64 userlands).
(plus on ia64, but that's irrelevant on musl)

Alexander

Re: [PATCH] add sched_getcpu

[nathan]

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

As far as I can tell, syscall() is supposed to set errno (using
__sycall_ret), but maybe src/misc/syscall.c isn't what I think it is?
I indeed got the return value backwards, and I'll fix that, along with the
#ifdef.

On Mon, Feb 29, 2016 at 6:23 PM Alexander Monakov <amonakov@ispras.ru>
wrote:

> On Mon, 29 Feb 2016, Nathan Zadoks wrote:
>
> > This is a GNU extension, but a fairly minor one, for a system call that
> > otherwise has no libc wrapper.
> >
> > Adding it was discussed previously, without any objections:
> > http://www.openwall.com/lists/musl/2015/05/08/24
> > ---
> >  include/sched.h          |  3 +++
> >  src/sched/sched_getcpu.c | 10 ++++++++++
> >  2 files changed, 13 insertions(+)
> >  create mode 100644 src/sched/sched_getcpu.c
> >
> > diff --git a/include/sched.h b/include/sched.h
> > index 3e34a72..17f5e06 100644
> > --- a/include/sched.h
> > +++ b/include/sched.h
> > @@ -76,6 +76,9 @@ void free(void *);
> >
> >  typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; }
> cpu_set_t;
> >  int __sched_cpucount(size_t, const cpu_set_t *);
> > +#ifdef _GNU_SOURCE
>
> This code is already under the same #ifdef.
>
> > +int sched_getcpu(void);
> > +#endif
> >  int sched_getaffinity(pid_t, size_t, cpu_set_t *);
> >  int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
> >
> > diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
> > new file mode 100644
> > index 0000000..070d6e7
> > --- /dev/null
> > +++ b/src/sched/sched_getcpu.c
> > @@ -0,0 +1,10 @@
> > +#define _GNU_SOURCE
> > +#include <stdlib.h>
>
> Do you need this include?
>
> > +#include <sched.h>
>
> (this include could also be dropped; I think it's a matter of policy
> whether
> such includes are desirable or not, so please wait for comment from Rich)
>
> > +#include "syscall.h"
> > +
> > +int sched_getcpu(void) {
> > +  int c, s;
> > +  s = syscall(SYS_getcpu, &c, NULL, NULL);
> > +  return (s == 0) ? c : s;
>
> This is wrong, as it doesn't set errno on error, and does not produce -1.
> This
> should be something like 'return s ? __syscall_ret(s) : c;' or maybe
> 'return __syscall_ret(s ? s : c);'.
>
> Alexander
>

[-- Attachment #2: Type: text/html, Size: 2871 bytes --]

Re: [PATCH] add sched_getcpu

[nathan]

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

On Mon, Feb 29, 2016 at 6:49 PM nathan@nathan7.eu <nathan@nathan7.eu> wrote:

> As far as I can tell, syscall() is supposed to set errno (using
> __sycall_ret), but maybe src/misc/syscall.c isn't what I think it is?
>
I indeed got the return value backwards, and I'll fix that, along with the
> #ifdef.
>
Actually, nope, that ternary is the right way round, and __sycall_ret
handles the -1 return.

>
>
> On Mon, Feb 29, 2016 at 6:23 PM Alexander Monakov <amonakov@ispras.ru>
> wrote:
>
>> On Mon, 29 Feb 2016, Nathan Zadoks wrote:
>>
>> > This is a GNU extension, but a fairly minor one, for a system call that
>> > otherwise has no libc wrapper.
>> >
>> > Adding it was discussed previously, without any objections:
>> > http://www.openwall.com/lists/musl/2015/05/08/24
>> > ---
>> >  include/sched.h          |  3 +++
>> >  src/sched/sched_getcpu.c | 10 ++++++++++
>> >  2 files changed, 13 insertions(+)
>> >  create mode 100644 src/sched/sched_getcpu.c
>> >
>> > diff --git a/include/sched.h b/include/sched.h
>> > index 3e34a72..17f5e06 100644
>> > --- a/include/sched.h
>> > +++ b/include/sched.h
>> > @@ -76,6 +76,9 @@ void free(void *);
>> >
>> >  typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; }
>> cpu_set_t;
>> >  int __sched_cpucount(size_t, const cpu_set_t *);
>> > +#ifdef _GNU_SOURCE
>>
>> This code is already under the same #ifdef.
>>
>> > +int sched_getcpu(void);
>> > +#endif
>> >  int sched_getaffinity(pid_t, size_t, cpu_set_t *);
>> >  int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
>> >
>> > diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
>> > new file mode 100644
>> > index 0000000..070d6e7
>> > --- /dev/null
>> > +++ b/src/sched/sched_getcpu.c
>> > @@ -0,0 +1,10 @@
>> > +#define _GNU_SOURCE
>> > +#include <stdlib.h>
>>
>> Do you need this include?
>>
>> > +#include <sched.h>
>>
>> (this include could also be dropped; I think it's a matter of policy
>> whether
>> such includes are desirable or not, so please wait for comment from Rich)
>>
>> > +#include "syscall.h"
>> > +
>> > +int sched_getcpu(void) {
>> > +  int c, s;
>> > +  s = syscall(SYS_getcpu, &c, NULL, NULL);
>> > +  return (s == 0) ? c : s;
>>
>> This is wrong, as it doesn't set errno on error, and does not produce -1.
>> This
>> should be something like 'return s ? __syscall_ret(s) : c;' or maybe
>> 'return __syscall_ret(s ? s : c);'.
>>
>> Alexander
>>
>

[-- Attachment #2: Type: text/html, Size: 3690 bytes --]

Re: [PATCH] add sched_getcpu

[Rich Felker]

On Mon, Feb 29, 2016 at 08:23:33PM +0300, Alexander Monakov wrote:
> > +int sched_getcpu(void);
> > +#endif
> >  int sched_getaffinity(pid_t, size_t, cpu_set_t *);
> >  int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
> >  
> > diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
> > new file mode 100644
> > index 0000000..070d6e7
> > --- /dev/null
> > +++ b/src/sched/sched_getcpu.c
> > @@ -0,0 +1,10 @@
> > +#define _GNU_SOURCE
> > +#include <stdlib.h>
> 
> Do you need this include?
> 
> > +#include <sched.h>
> 
> (this include could also be dropped; I think it's a matter of policy whether
> such includes are desirable or not, so please wait for comment from Rich)

Policy is to always include the header with the public declaration
(and any feature test macros necessary to get it) so that the compiler
checks the implementation against the public declaration.

> > +#include "syscall.h"
> > +
> > +int sched_getcpu(void) {
> > +  int c, s;
> > +  s = syscall(SYS_getcpu, &c, NULL, NULL);
> > +  return (s == 0) ? c : s;
> 
> This is wrong, as it doesn't set errno on error, and does not produce -1. This
> should be something like 'return s ? __syscall_ret(s) : c;' or maybe
> 'return __syscall_ret(s ? s : c);'.

Why is an error even possible here? Is it just to account for ancient
kernels that lack the syscall?

Rich

Re: [PATCH] add sched_getcpu

[Alexander Monakov]

On Mon, 29 Feb 2016, Rich Felker wrote:
> > (this include could also be dropped; I think it's a matter of policy whether
> > such includes are desirable or not, so please wait for comment from Rich)
> 
> Policy is to always include the header with the public declaration
> (and any feature test macros necessary to get it) so that the compiler
> checks the implementation against the public declaration.

This policy certain makes sense; I pointed that out because I've seen it
violated; at least the following files violate it by defining something
without including anything:

arch/arm/src/__aeabi_atexit.c
src/internal/procfdname.c
src/misc/gethostid.c
src/prng/__seed48.c
src/signal/restore.c
src/signal/sigrtmin.c
src/stdlib/abs.c
src/stdlib/labs.c
src/stdlib/llabs.c
src/time/__month_to_secs.c
src/time/__year_to_secs.c

(but e.g. in procfdname.c there's nothing to include because every caller
declares the prototype, and also hardcodes the buffer size -- see my recent
patch)

> > This is wrong, as it doesn't set errno on error, and does not produce -1. This
> > should be something like 'return s ? __syscall_ret(s) : c;' or maybe
> > 'return __syscall_ret(s ? s : c);'.
> 
> Why is an error even possible here? Is it just to account for ancient
> kernels that lack the syscall?

Apparently so -- afaics current kernels can only produce EFAULT, which cannot
happen in this case.

(I was wrong about the need to use __syscall_ret, but that's for another
subthread)

Alexander

Re: [PATCH] add sched_getcpu

[Rich Felker]

On Mon, Feb 29, 2016 at 10:59:10PM +0300, Alexander Monakov wrote:
> On Mon, 29 Feb 2016, Rich Felker wrote:
> > > (this include could also be dropped; I think it's a matter of policy whether
> > > such includes are desirable or not, so please wait for comment from Rich)
> > 
> > Policy is to always include the header with the public declaration
> > (and any feature test macros necessary to get it) so that the compiler
> > checks the implementation against the public declaration.
> 
> This policy certain makes sense; I pointed that out because I've seen it
> violated; at least the following files violate it by defining something
> without including anything:

Thanks for tracking these down. See below:

> arch/arm/src/__aeabi_atexit.c

This ia an ABI function but has no public declaration and is not
callable as API.

> src/internal/procfdname.c

This is an internal function.

> src/misc/gethostid.c

Should be fixed to include unistd.h.

> src/prng/__seed48.c
> src/signal/restore.c

Internal.

> src/signal/sigrtmin.c

ABI but the symbol is exposed via a public macro in signal.h so I
think we should include the header.

> src/stdlib/abs.c
> src/stdlib/labs.c
> src/stdlib/llabs.c

Definitely should include header.

> src/time/__month_to_secs.c
> src/time/__year_to_secs.c

Internal.

Rich

Re: [PATCH] add sched_getcpu

[Alexander Monakov]

On Mon, 29 Feb 2016, Rich Felker wrote:
> > > Policy is to always include the header with the public declaration
> > > (and any feature test macros necessary to get it) so that the compiler
> > > checks the implementation against the public declaration.
> > 
> > This policy certain makes sense; I pointed that out because I've seen it
> > violated; at least the following files violate it by defining something
> > without including anything:
[snip]
> > src/internal/procfdname.c
> 
> This is an internal function.

Please explain the difference in policy for internal functions. The original
motivation (compiler checking the prototype) sounds like it's valuable for
internal functions too.

Thanks.
Alexander

Re: [PATCH] add sched_getcpu

[Alexander Monakov]

> > As far as I can tell, syscall() is supposed to set errno (using
> > __sycall_ret), but maybe src/misc/syscall.c isn't what I think it is?
> >
> I indeed got the return value backwards, and I'll fix that, along with the
> > #ifdef.
> >
> Actually, nope, that ternary is the right way round, and __sycall_ret
> handles the -1 return.

Ah, indeed my original objection was wrong; however please note that you're
not calling syscall from src/misc/syscall.c here; it's #defined in
src/internal/syscall.h to __syscall_ret(__syscall(...)), and the typical usage
in musl internally is either tailcalling to syscall, or calling __syscall
where handling provided by __syscall_ret is not useful.

I'd recommend using __syscall and __syscall_ret here.

Alexander

Re: [PATCH] add sched_getcpu

[Rich Felker]

On Mon, Feb 29, 2016 at 11:10:51PM +0300, Alexander Monakov wrote:
> On Mon, 29 Feb 2016, Rich Felker wrote:
> > > > Policy is to always include the header with the public declaration
> > > > (and any feature test macros necessary to get it) so that the compiler
> > > > checks the implementation against the public declaration.
> > > 
> > > This policy certain makes sense; I pointed that out because I've seen it
> > > violated; at least the following files violate it by defining something
> > > without including anything:
> [snip]
> > > src/internal/procfdname.c
> > 
> > This is an internal function.
> 
> Please explain the difference in policy for internal functions. The original
> motivation (compiler checking the prototype) sounds like it's valuable for
> internal functions too.

I agree there's value to both, but as stated ("...public
declaration...") the policy only applies to public APIs. Part of the
difference that makes it more important is that we're actually
checking the correctness of the prototype, which is an error that
would leak into programs compiled against musl's headers if it's
wrong, whereas for internal functions the bug from a mismatch is at
worst an internal bug.

With that said I agree it would be nice to have prototypes checked for
internal functions too, but some of them don't have a natural place to
put the prototype without adding gratuitous tiny header files or
lumping more stuff into the existing internal headers. This is
probably an area that could use some thought to cleanup.

Rich

[PATCH] add sched_getcpu

[Nathan Zadoks]

This is a GNU extension, but a fairly minor one, for a system call that
otherwise has no libc wrapper.

Adding it was discussed previously, without any objections:
http://www.openwall.com/lists/musl/2015/05/08/24
---
 include/sched.h          |  3 +++
 src/sched/sched_getcpu.c | 11 +++++++++++
 2 files changed, 14 insertions(+)
 create mode 100644 src/sched/sched_getcpu.c

diff --git a/include/sched.h b/include/sched.h
index 3e34a72..17f5e06 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -76,6 +76,9 @@ void free(void *);
 
 typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; } cpu_set_t;
 int __sched_cpucount(size_t, const cpu_set_t *);
+#ifdef _GNU_SOURCE
+int sched_getcpu(void);
+#endif
 int sched_getaffinity(pid_t, size_t, cpu_set_t *);
 int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
 
diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
new file mode 100644
index 0000000..f34d84a
--- /dev/null
+++ b/src/sched/sched_getcpu.c
@@ -0,0 +1,11 @@
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <sched.h>
+#include "syscall.h"
+
+int sched_getcpu(void)
+{
+  int c, s;
+  s = __syscall(SYS_getcpu, &c, NULL, NULL);
+  return __syscall_ret((s == 0) ? c : s);
+}
-- 
2.7.1

Re: [PATCH] add sched_getcpu

[Tomasz Sterna]

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

W dniu 29.02.2016, pon o godzinie 17∶49 +0100, użytkownik Nathan Zadoks
napisał:
> This is a GNU extension, but a fairly minor one, for a system call
> that otherwise has no libc wrapper.

Does it need a libc wrapper?
Calling it using syscall() seems pretty straightforward.

There are a lot of Linux specific syscalls without libc wrappers.
Is this one special enough?


-- 
 /o__ Q: What is orange and goes "click, click?"
(_<^' A: A ball point carrot.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add sched_getcpu

[Nathan Zadoks]

No objections have been noted previously, and in addition this has a 
vDSO equivalent on on x86_64, x32, and 64-bit ppc kernels. I haven't 
added support for that yet, but I'm hoping to make that my next patch if 
this gets merged.
Shoving the raw syscall into every app seems unproductive, and adding 
vDSO handling to random userland apps seems especially odd.
I *started* this patch because a patch of mine to support musl in an 
application was denied, on grounds of the syscall handling belonging in 
libc.

On 29/02/16 22:09, Tomasz Sterna wrote:
> W dniu 29.02.2016, pon o godzinie 17∶49 +0100, użytkownik Nathan Zadoks
> napisał:
>> This is a GNU extension, but a fairly minor one, for a system call
>> that otherwise has no libc wrapper.
> Does it need a libc wrapper?
> Calling it using syscall() seems pretty straightforward.
>
> There are a lot of Linux specific syscalls without libc wrappers.
> Is this one special enough?
>
>

Re: [PATCH] add sched_getcpu

[Rich Felker]

On Mon, Feb 29, 2016 at 10:09:54PM +0100, Tomasz Sterna wrote:
> W dniu 29.02.2016, pon o godzinie 17∶49 +0100, użytkownik Nathan Zadoks
> napisał:
> > This is a GNU extension, but a fairly minor one, for a system call
> > that otherwise has no libc wrapper.
> 
> Does it need a libc wrapper?
> Calling it using syscall() seems pretty straightforward.

If getting sufficient performance for it to be practically useful ends
up dependent on vdso, then calling it via syscall() is not useful at
all.

> There are a lot of Linux specific syscalls without libc wrappers.
> Is this one special enough?

I wouldn't say there are a lot; do you have a list? The topic of what
should and should not be given libc wrappers has been under discussion
on the glibc list for a while now. There's been some strong opposition
to exposing new things, even useful ones, if they're clearly
"Linux-specific", but my position has been that libc is justified in
exposing anything that can reasonably be used by applications without
making assumptions about libc internals. This means things like
set_thread_area, set_tid_address, set_robust_list, brk, possibly
gettid, etc. are not appropriate to expose, but other things could be.

You should not need to use syscall() to access any Linux functionality
that's meant to be exposed to applications; syscall() has nasty quirks
that vary by arch (like alignment of 64-bit argument slots, argument
passing on x32 and similar ILP32-on-64 ABIs, etc.) that applications
should never have to be aware of.

Rich

[PATCH] add sched_getcpu, with vDSO support

[Nathan Zadoks]

This is a GNU extension, but a fairly minor one, for a system call that
otherwise has no libc wrapper.

Adding it was discussed previously, without any objections:
http://www.openwall.com/lists/musl/2015/05/08/24
---

I've done some cleanup and added vDSO support, since it seems to make a
significant perf difference.  (about 14ns/vDSO call, vs about 102ns/syscall)
I've only added vDSO support for x86_64, since it doesn't exist on i386, and I
don't see any vDSO handling at all on x32.
The vDSO function is also available on PowerPC, but I don't have any machines
for testing that on, so I've left that out too.

---

 arch/x86_64/syscall_arch.h |  2 ++
 include/sched.h            |  1 +
 src/sched/sched_getcpu.c   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 src/sched/sched_getcpu.c

diff --git a/arch/x86_64/syscall_arch.h b/arch/x86_64/syscall_arch.h
index a7a7b5a..54e05ff 100644
--- a/arch/x86_64/syscall_arch.h
+++ b/arch/x86_64/syscall_arch.h
@@ -64,3 +64,5 @@ static __inline long __syscall6(long n, long a1, long a2, long a3, long a4, long
 #define VDSO_USEFUL
 #define VDSO_CGT_SYM "__vdso_clock_gettime"
 #define VDSO_CGT_VER "LINUX_2.6"
+#define VDSO_GETCPU_SYM "__vdso_getcpu"
+#define VDSO_GETCPU_VER "LINUX_2.6"
diff --git a/include/sched.h b/include/sched.h
index 3e34a72..7e88f09 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -76,6 +76,7 @@ void free(void *);
 
 typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; } cpu_set_t;
 int __sched_cpucount(size_t, const cpu_set_t *);
+int sched_getcpu(void);
 int sched_getaffinity(pid_t, size_t, cpu_set_t *);
 int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
 
diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
new file mode 100644
index 0000000..ed5e331
--- /dev/null
+++ b/src/sched/sched_getcpu.c
@@ -0,0 +1,45 @@
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <errno.h>
+#include <sched.h>
+#include "syscall.h"
+#include "atomic.h"
+
+#ifdef VDSO_GETCPU_SYM
+
+void *__vdsosym(const char *, const char *);
+
+static void *volatile vdso_func;
+
+typedef long (*getcpu_f)(unsigned *, unsigned *, void *);
+
+long getcpu_init(unsigned *cpu, unsigned *node, void *unused)
+{
+  void *p = __vdsosym(VDSO_GETCPU_VER, VDSO_GETCPU_SYM);
+  getcpu_f f = (getcpu_f)p;
+  a_cas_p(&vdso_func, (void *)getcpu_init, p);
+  return f ? f(cpu, node, unused) : -ENOSYS;
+}
+
+static void *volatile vdso_func = (void *)getcpu_init;
+
+#endif
+
+int sched_getcpu(void)
+{
+  int r;
+  unsigned cpu;
+
+#ifdef VDSO_CGT_SYM
+  getcpu_f f = (getcpu_f)vdso_func;
+  if (f) {
+    r = f(&cpu, NULL, NULL);
+    if (!r) return cpu;
+    if (r != -ENOSYS) return __syscall_ret(r);
+  }
+#endif
+
+  r = __syscall(SYS_getcpu, &cpu, NULL, NULL);
+  if (!r) return cpu;
+  return __syscall_ret(r);
+}
-- 
2.7.1

[PATCH] add sched_getcpu, with vDSO support

[Nathan Zadoks]

This is a GNU extension, but a fairly minor one, for a system call that
otherwise has no libc wrapper.

Adding it was discussed previously, without any objections:
http://www.openwall.com/lists/musl/2015/05/08/24
---

Whoops, forgot to make getcpu_init a static function in the previous version.
Fixed!

---

 arch/x86_64/syscall_arch.h |  2 ++
 include/sched.h            |  1 +
 src/sched/sched_getcpu.c   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 src/sched/sched_getcpu.c

diff --git a/arch/x86_64/syscall_arch.h b/arch/x86_64/syscall_arch.h
index a7a7b5a..54e05ff 100644
--- a/arch/x86_64/syscall_arch.h
+++ b/arch/x86_64/syscall_arch.h
@@ -64,3 +64,5 @@ static __inline long __syscall6(long n, long a1, long a2, long a3, long a4, long
 #define VDSO_USEFUL
 #define VDSO_CGT_SYM "__vdso_clock_gettime"
 #define VDSO_CGT_VER "LINUX_2.6"
+#define VDSO_GETCPU_SYM "__vdso_getcpu"
+#define VDSO_GETCPU_VER "LINUX_2.6"
diff --git a/include/sched.h b/include/sched.h
index 3e34a72..7e88f09 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -76,6 +76,7 @@ void free(void *);
 
 typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; } cpu_set_t;
 int __sched_cpucount(size_t, const cpu_set_t *);
+int sched_getcpu(void);
 int sched_getaffinity(pid_t, size_t, cpu_set_t *);
 int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
 
diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
new file mode 100644
index 0000000..096f06f
--- /dev/null
+++ b/src/sched/sched_getcpu.c
@@ -0,0 +1,45 @@
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <errno.h>
+#include <sched.h>
+#include "syscall.h"
+#include "atomic.h"
+
+#ifdef VDSO_GETCPU_SYM
+
+void *__vdsosym(const char *, const char *);
+
+static void *volatile vdso_func;
+
+typedef long (*getcpu_f)(unsigned *, unsigned *, void *);
+
+static long getcpu_init(unsigned *cpu, unsigned *node, void *unused)
+{
+  void *p = __vdsosym(VDSO_GETCPU_VER, VDSO_GETCPU_SYM);
+  getcpu_f f = (getcpu_f)p;
+  a_cas_p(&vdso_func, (void *)getcpu_init, p);
+  return f ? f(cpu, node, unused) : -ENOSYS;
+}
+
+static void *volatile vdso_func = (void *)getcpu_init;
+
+#endif
+
+int sched_getcpu(void)
+{
+  int r;
+  unsigned cpu;
+
+#ifdef VDSO_CGT_SYM
+  getcpu_f f = (getcpu_f)vdso_func;
+  if (f) {
+    r = f(&cpu, NULL, NULL);
+    if (!r) return cpu;
+    if (r != -ENOSYS) return __syscall_ret(r);
+  }
+#endif
+
+  r = __syscall(SYS_getcpu, &cpu, NULL, NULL);
+  if (!r) return cpu;
+  return __syscall_ret(r);
+}
-- 
2.7.1

Re: [PATCH] add sched_getcpu

[Tomasz Sterna]

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

W dniu 29.02.2016, pon o godzinie 16∶30 -0500, użytkownik Rich Felker
napisał:
> There are a lot of Linux specific syscalls without libc wrappers.
> > Is this one special enough?
> I wouldn't say there are a lot; do you have a list?

$ grep -roh 'syscall(SYS_[a-z]*' src/kits/kernel/ | sort -u
syscall(SYS_exit
syscall(SYS_futex
syscall(SYS_gettid
syscall(SYS_tgkill

And this is just from a fairly small library of mine.


>  The topic of what should and should not be given libc wrappers has
> been under discussion on the glibc list for a while now. [...]
> You should not need to use syscall() to access any Linux
> functionality that's meant to be exposed to applications; [...]

Really depends on how you define "applications".

Basically why I asked the question - if the above should also be
wrapped, I am all for it. But if not, why some syscalls are special?


-- 
smoku @ http://abadcafe.pl/ @ http://xiaoka.com/


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add sched_getcpu

[Rich Felker]

On Tue, Mar 01, 2016 at 09:35:21PM +0100, Tomasz Sterna wrote:
> W dniu 29.02.2016, pon o godzinie 16∶30 -0500, użytkownik Rich Felker
> napisał:
> > There are a lot of Linux specific syscalls without libc wrappers.
> > > Is this one special enough?
> > I wouldn't say there are a lot; do you have a list?
> 
> $ grep -roh 'syscall(SYS_[a-z]*' src/kits/kernel/ | sort -u
> syscall(SYS_exit

SYS_exit cannot be used safely unless you have a single-threaded
program, and in that case you can use _exit (SYS_exit_group).

> syscall(SYS_futex

There's work on getting glibc to expose futex, but I want to ensure
that we do it in a compatible (in terms of types used, and where the
declaration and macros get defined) way, so I'm waiting til we reach
consensus on that to take any action in musl.

> syscall(SYS_gettid

For glibc it's been controversial whether to expose tids as a public
API, since it pokes through the pthread abstraction and imposes a 1:1
threads implementation. My view is that, unless you want to write a
gigantic framework emulating each blocking syscall in userspace, POSIX
already imposes a 1:1 threads implementation, and it's stupid to still
pretend that M:N is viable.

> syscall(SYS_tgkill

tgkill also requires tids to be exposed an potentially has other
issues, and doesn't seem to offer anything that pthread_kill doesn't.

> >  The topic of what should and should not be given libc wrappers has
> > been under discussion on the glibc list for a while now. [...]
> > You should not need to use syscall() to access any Linux
> > functionality that's meant to be exposed to applications; [...]
> 
> Really depends on how you define "applications".

I mean syscalls that are intended for writing application programs
rather than for implementing the userspace part of C/POSIX.

> Basically why I asked the question - if the above should also be
> wrapped, I am all for it. But if not, why some syscalls are special?

I hope I've answered this to some extent. If not please elaborate on
what specific things aren't clear.

Rich

Re: [PATCH] add sched_getcpu, with vDSO support

[Rich Felker]

On Tue, Mar 01, 2016 at 04:56:06PM +0100, Nathan Zadoks wrote:
> This is a GNU extension, but a fairly minor one, for a system call that
> otherwise has no libc wrapper.
> 
> Adding it was discussed previously, without any objections:
> http://www.openwall.com/lists/musl/2015/05/08/24
> ---
> 
> Whoops, forgot to make getcpu_init a static function in the previous version.
> Fixed!
> 
> ---
> 
>  arch/x86_64/syscall_arch.h |  2 ++
>  include/sched.h            |  1 +
>  src/sched/sched_getcpu.c   | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 48 insertions(+)
>  create mode 100644 src/sched/sched_getcpu.c
> 
> diff --git a/arch/x86_64/syscall_arch.h b/arch/x86_64/syscall_arch.h
> index a7a7b5a..54e05ff 100644
> --- a/arch/x86_64/syscall_arch.h
> +++ b/arch/x86_64/syscall_arch.h
> @@ -64,3 +64,5 @@ static __inline long __syscall6(long n, long a1, long a2, long a3, long a4, long
>  #define VDSO_USEFUL
>  #define VDSO_CGT_SYM "__vdso_clock_gettime"
>  #define VDSO_CGT_VER "LINUX_2.6"
> +#define VDSO_GETCPU_SYM "__vdso_getcpu"
> +#define VDSO_GETCPU_VER "LINUX_2.6"
> diff --git a/include/sched.h b/include/sched.h
> index 3e34a72..7e88f09 100644
> --- a/include/sched.h
> +++ b/include/sched.h
> @@ -76,6 +76,7 @@ void free(void *);
>  
>  typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; } cpu_set_t;
>  int __sched_cpucount(size_t, const cpu_set_t *);
> +int sched_getcpu(void);
>  int sched_getaffinity(pid_t, size_t, cpu_set_t *);
>  int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
>  
> diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
> new file mode 100644
> index 0000000..096f06f
> --- /dev/null
> +++ b/src/sched/sched_getcpu.c
> @@ -0,0 +1,45 @@
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <sched.h>
> +#include "syscall.h"
> +#include "atomic.h"
> +
> +#ifdef VDSO_GETCPU_SYM
> +
> +void *__vdsosym(const char *, const char *);
> +
> +static void *volatile vdso_func;
> +
> +typedef long (*getcpu_f)(unsigned *, unsigned *, void *);
> +
> +static long getcpu_init(unsigned *cpu, unsigned *node, void *unused)
> +{
> +  void *p = __vdsosym(VDSO_GETCPU_VER, VDSO_GETCPU_SYM);
> +  getcpu_f f = (getcpu_f)p;
> +  a_cas_p(&vdso_func, (void *)getcpu_init, p);
> +  return f ? f(cpu, node, unused) : -ENOSYS;
> +}
> +
> +static void *volatile vdso_func = (void *)getcpu_init;
> +
> +#endif
> +
> +int sched_getcpu(void)
> +{
> +  int r;
> +  unsigned cpu;
> +
> +#ifdef VDSO_CGT_SYM

Wrong macro in the #ifdef.

> +  getcpu_f f = (getcpu_f)vdso_func;
> +  if (f) {
> +    r = f(&cpu, NULL, NULL);

Not a big deal, but usually in musl we use 0 rather than the NULL
macro.

Actually I wondered if the function actually needs to take the useless
extra 2 arguments, but I think for the sake of correctness it's best
to do it this way -- the callee in the vdso has 3 args, so it should
be called with the correct type.

> +    if (!r) return cpu;
> +    if (r != -ENOSYS) return __syscall_ret(r);
> +  }
> +#endif
> +
> +  r = __syscall(SYS_getcpu, &cpu, NULL, NULL);
> +  if (!r) return cpu;
> +  return __syscall_ret(r);
> +}

Also tabs should be used for indention, not spaces.

One other thing I thought might be nice is initially committing the
trivial syscall-only version that adds the public prototype, then
doing vdso support as a separate commit, but if that's a pain to do
don't worry about it.

Rich

[PATCH 0/2] add sched_getcpu, take n+1

[Nathan Zadoks]

> Wrong macro in the #ifdef.
Whoops, thanks, fixed!

> Not a big deal, but usually in musl we use 0 rather than the NULL
> macro.
Learnt a new thing there - fixed!

> Actually I wondered if the function actually needs to take the useless
> extra 2 arguments, but I think for the sake of correctness it's best
> to do it this way -- the callee in the vdso has 3 args, so it should
> be called with the correct type.
The second argument actually needs to be null: it's an out pointer for
an identifier for the NUMA node. sched_getcpu doesn't have a spot for
returning it, so we pass a null pointer there, which ignores it.
The *third* argument is a pointer to a now-unused cache structure,
and could probably be omitted safely. It's required to be non-NULL on kernels
older than 2.6.23, but we'd just end up corrupting random memory there
instead of getting an EFAULT. Not worth saving a register zeroing instruction.

> Also tabs should be used for indention, not spaces.
Yup, noticed that one before, was saving the fix for the next patch email.

> One other thing I thought might be nice is initially committing the
> trivial syscall-only version that adds the public prototype, then
> doing vdso support as a separate commit, but if that's a pain to do
> don't worry about it.
No problem! Some git trickery later, I've got this sorted.

Patches coming up in the next two emails, I'm still figuring out this
git-send-email thing.

Nathan Zadoks (2):
  add sched_getcpu
  add sched_getcpu vDSO support

 arch/x86_64/syscall_arch.h |  2 ++
 include/sched.h            |  1 +
 src/sched/sched_getcpu.c   | 44 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)
 create mode 100644 src/sched/sched_getcpu.c

-- 
2.7.1

[PATCH 1/2] add sched_getcpu

[Nathan Zadoks]

This is a GNU extension, but a fairly minor one, for a system call that
otherwise has no libc wrapper.

Adding it was discussed previously, without any objections:
http://www.openwall.com/lists/musl/2015/05/08/24
---
 include/sched.h          |  1 +
 src/sched/sched_getcpu.c | 13 +++++++++++++
 2 files changed, 14 insertions(+)
 create mode 100644 src/sched/sched_getcpu.c

diff --git a/include/sched.h b/include/sched.h
index 3e34a72..7e88f09 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -76,6 +76,7 @@ void free(void *);
 
 typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)]; } cpu_set_t;
 int __sched_cpucount(size_t, const cpu_set_t *);
+int sched_getcpu(void);
 int sched_getaffinity(pid_t, size_t, cpu_set_t *);
 int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
 
diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
new file mode 100644
index 0000000..760e4d5
--- /dev/null
+++ b/src/sched/sched_getcpu.c
@@ -0,0 +1,13 @@
+#define _GNU_SOURCE
+#include <sched.h>
+#include "syscall.h"
+
+int sched_getcpu(void)
+{
+	int r;
+	unsigned cpu;
+
+	r = __syscall(SYS_getcpu, &cpu, 0, 0);
+	if (!r) return cpu;
+	return __syscall_ret(r);
+}
-- 
2.7.1

[PATCH 2/2] add sched_getcpu vDSO support

[Nathan Zadoks]

This brings the call to an actually usable speed.
Quick unscientific benchmark: 14ns : 102ns :: vDSO : syscall
---
 arch/x86_64/syscall_arch.h |  2 ++
 src/sched/sched_getcpu.c   | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/arch/x86_64/syscall_arch.h b/arch/x86_64/syscall_arch.h
index a7a7b5a..54e05ff 100644
--- a/arch/x86_64/syscall_arch.h
+++ b/arch/x86_64/syscall_arch.h
@@ -64,3 +64,5 @@ static __inline long __syscall6(long n, long a1, long a2, long a3, long a4, long
 #define VDSO_USEFUL
 #define VDSO_CGT_SYM "__vdso_clock_gettime"
 #define VDSO_CGT_VER "LINUX_2.6"
+#define VDSO_GETCPU_SYM "__vdso_getcpu"
+#define VDSO_GETCPU_VER "LINUX_2.6"
diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
index 760e4d5..e08cfdf 100644
--- a/src/sched/sched_getcpu.c
+++ b/src/sched/sched_getcpu.c
@@ -1,12 +1,43 @@
 #define _GNU_SOURCE
+#include <errno.h>
 #include <sched.h>
 #include "syscall.h"
+#include "atomic.h"
+
+#ifdef VDSO_GETCPU_SYM
+
+void *__vdsosym(const char *, const char *);
+
+static void *volatile vdso_func;
+
+typedef long (*getcpu_f)(unsigned *, unsigned *, void *);
+
+static long getcpu_init(unsigned *cpu, unsigned *node, void *unused)
+{
+	void *p = __vdsosym(VDSO_GETCPU_VER, VDSO_GETCPU_SYM);
+	getcpu_f f = (getcpu_f)p;
+	a_cas_p(&vdso_func, (void *)getcpu_init, p);
+	return f ? f(cpu, node, unused) : -ENOSYS;
+}
+
+static void *volatile vdso_func = (void *)getcpu_init;
+
+#endif
 
 int sched_getcpu(void)
 {
 	int r;
 	unsigned cpu;
 
+#ifdef VDSO_GETCPU_SYM
+	getcpu_f f = (getcpu_f)vdso_func;
+	if (f) {
+		r = f(&cpu, 0, 0);
+		if (!r) return cpu;
+		if (r != -ENOSYS) return __syscall_ret(r);
+	}
+#endif
+
 	r = __syscall(SYS_getcpu, &cpu, 0, 0);
 	if (!r) return cpu;
 	return __syscall_ret(r);
-- 
2.7.1

Re: [PATCH] add sched_getcpu

[Tomasz Sterna]

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

W dniu 01.03.2016, wto o godzinie 17∶34 -0500, użytkownik Rich Felker
napisał:
> syscall(SYS_exit
> SYS_exit cannot be used safely unless you have a single-threaded
> program, and in that case you can use _exit (SYS_exit_group).

How should I properly terminate current task then?


> > syscall(SYS_gettid
> For glibc it's been controversial whether to expose tids as a public
> API, since it pokes through the pthread abstraction and imposes a 1:1
> threads implementation.

I am implementing a threading and mutex API that is different to
pthread. (Still 1:1 though.)
Using pthread to do this proved to be cumbersome, but using native
Linux abstractions turned out to be pretty straightforward.


> syscall(SYS_tgkill
> tgkill also requires tids to be exposed an potentially has other
> issues, and doesn't seem to offer anything that pthread_kill doesn't.

As above - using pthreads is not the good way to do it in my case.


> wrapped, I am all for it. But if not, why some syscalls are
> > special?
> I hope I've answered this to some extent.

More than enough.
Thank you for your patience.


-- 
 /o__ 
(_<^' "Rome wasn't burned in a day. "


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add sched_getcpu

[Szabolcs Nagy]

* Tomasz Sterna <tomek@xiaoka.com> [2016-03-02 21:46:53 +0100]:
> W dniu 01.03.2016, wto o godzinie 17???34 -0500, u??ytkownik Rich Felker
> napisa??:
> > syscall(SYS_exit
> > SYS_exit cannot be used safely unless you have a single-threaded
> > program, and in that case you can use _exit (SYS_exit_group).
> 
> How should I properly terminate current task then?
> 

pthread_exit

> > > syscall(SYS_gettid
> > For glibc it's been controversial whether to expose tids as a public
> > API, since it pokes through the pthread abstraction and imposes a 1:1
> > threads implementation.
> 
> I am implementing a threading and mutex API that is different to
> pthread. (Still 1:1 though.)
> Using pthread to do this proved to be cumbersome, but using native
> Linux abstractions turned out to be pretty straightforward.
> 

that's not possible in c

the semantics (memory model, libc internals..) assume
that threads can only be created by the c runtime.

in theory you can create you own threads, but you have
to know what you are doing (no libc calls, no tls)
but then you are implementing your own libc

> > syscall(SYS_tgkill
> > tgkill also requires tids to be exposed an potentially has other
> > issues, and doesn't seem to offer anything that pthread_kill doesn't.
> 
> As above - using pthreads is not the good way to do it in my case.
> 

what you are doing is undefined behaviour.

Re: [PATCH] add sched_getcpu

[Rich Felker]

On Wed, Mar 02, 2016 at 10:19:25PM +0100, Szabolcs Nagy wrote:
> > > > syscall(SYS_gettid
> > > For glibc it's been controversial whether to expose tids as a public
> > > API, since it pokes through the pthread abstraction and imposes a 1:1
> > > threads implementation.
> > 
> > I am implementing a threading and mutex API that is different to
> > pthread. (Still 1:1 though.)
> > Using pthread to do this proved to be cumbersome, but using native
> > Linux abstractions turned out to be pretty straightforward.
> > 
> 
> that's not possible in c
> 
> the semantics (memory model, libc internals..) assume
> that threads can only be created by the c runtime.
> 
> in theory you can create you own threads, but you have
> to know what you are doing (no libc calls, no tls)
> but then you are implementing your own libc

Indeed, on any modern libc, a thread not created by the standard
functions cannot call any standard library function safely. This is
because (among other reasons) the thread pointer (used for thread
local storage) needs to point to a correctly setup data structure
whose definition is not public and which may vary between libc
builds/versions. musl is somewhat more conservative about using the
thread pointer internally (because it can be slow on some archs, and
because historically we supported pre-TLS kernels) but things will
break immediately on glibc if you break this rule, and fairly quickly
on musl too.

The only safe way to make your own threads is to refrain from using
libc at all and do your own syscalls. Even calling syscall() may not
be safe (on i386/glibc it probably uses the vdso syscall pointer from
TLS); you really need to use asm to make the syscall.

> > > syscall(SYS_tgkill
> > > tgkill also requires tids to be exposed an potentially has other
> > > issues, and doesn't seem to offer anything that pthread_kill doesn't.
> > 
> > As above - using pthreads is not the good way to do it in my case.
> 
> what you are doing is undefined behaviour.

From a standards perspective it's just outside the scope of what's
defined. From musl's and glibc's perspectives, the behavior is
undefined.

Is there a reason "pthreads is not a good way to do it"?

Rich

Re: [PATCH 0/2] add sched_getcpu, take n+1

[Rich Felker]

On Wed, Mar 02, 2016 at 05:26:25PM +0100, Nathan Zadoks wrote:
> > Wrong macro in the #ifdef.
> Whoops, thanks, fixed!
> 
> > Not a big deal, but usually in musl we use 0 rather than the NULL
> > macro.
> Learnt a new thing there - fixed!
> 
> > Actually I wondered if the function actually needs to take the useless
> > extra 2 arguments, but I think for the sake of correctness it's best
> > to do it this way -- the callee in the vdso has 3 args, so it should
> > be called with the correct type.
> The second argument actually needs to be null: it's an out pointer for
> an identifier for the NUMA node. sched_getcpu doesn't have a spot for
> returning it, so we pass a null pointer there, which ignores it.
> The *third* argument is a pointer to a now-unused cache structure,
> and could probably be omitted safely. It's required to be non-NULL on kernels
> older than 2.6.23, but we'd just end up corrupting random memory there
> instead of getting an EFAULT. Not worth saving a register zeroing instruction.

Agreed.

> > One other thing I thought might be nice is initially committing the
> > trivial syscall-only version that adds the public prototype, then
> > doing vdso support as a separate commit, but if that's a pain to do
> > don't worry about it.
> No problem! Some git trickery later, I've got this sorted.

Great. Applying now. Thanks!

Rich

Re: [PATCH] add sched_getcpu

[Tomasz Sterna]

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

W dniu 02.03.2016, śro o godzinie 18∶26 -0500, użytkownik Rich Felker
napisał:
> The only safe way to make your own threads is to refrain from using
> libc at all and do your own syscalls. Even calling syscall() may not
> be safe (on i386/glibc it probably uses the vdso syscall pointer from
> TLS); you really need to use asm to make the syscall.

I understand.
Thank you for explaining.


> Is there a reason "pthreads is not a good way to do it"?

The thread API I am implementing differs much to pthreads, which
required a lot of glue code translating one API to another and felt
hand wrenching.
On the other hand implementing it on bare syscalls was refreshingly
simple.

But if I do not want to implement the whole libc with it, I will need
to revisit this idea.
C'est la vie...


-- 
 /o__ 
(_<^' The man on tops walks a lonely street; the "chain" of command is often a noose.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add sched_getcpu

[Rich Felker]

On Fri, Mar 04, 2016 at 11:21:49PM +0100, Tomasz Sterna wrote:
> W dniu 02.03.2016, śro o godzinie 18∶26 -0500, użytkownik Rich Felker
> napisał:
> > The only safe way to make your own threads is to refrain from using
> > libc at all and do your own syscalls. Even calling syscall() may not
> > be safe (on i386/glibc it probably uses the vdso syscall pointer from
> > TLS); you really need to use asm to make the syscall.
> 
> I understand.
> Thank you for explaining.
> 
> 
> > Is there a reason "pthreads is not a good way to do it"?
> 
> The thread API I am implementing differs much to pthreads, which
> required a lot of glue code translating one API to another and felt
> hand wrenching.

Are you sure? The only functions you really need to use pthreads are
pthread_create and one of either pthread_join or pthread_detach.
Beyond that, how you do your synchronization is up to you; the
standard functions are convenient but you're free to roll your own in
terms of atomics and futex or whatever.

Rich

Re: [PATCH] add sched_getcpu

[Tomasz Sterna]

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

W dniu 04.03.2016, pią o godzinie 18∶33 -0500, użytkownik Rich Felker
napisał:
> The thread API I am implementing differs much to pthreads, which
> > required a lot of glue code translating one API to another and felt
> > hand wrenching.
> Are you sure? The only functions you really need to use pthreads are
> pthread_create and one of either pthread_join or pthread_detach.

Well... I still have the source of the previous attempt of implementing
this API over standard glibc on disk. And it is ugly.
But it is possible that it's just a matter of this particular
implementation. Unfortunately the original author is MIA.

With the experience on doing it on raw clone and futex and knowledge
you just provided in this thread, it should not be that ugly this time
around. :)



-- 
 /o__ FORTUNE PROVIDES QUESTIONS FOR THE GREAT ANSWERS: #31
(_<^' A: Chicken Teriyaki.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]