From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/9406 Path: news.gmane.org!not-for-mail From: "nathan@nathan7.eu" Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] add sched_getcpu Date: Mon, 29 Feb 2016 17:49:13 +0000 Message-ID: References: <1456765028-23958-1-git-send-email-nathan@nathan7.eu> <1456765216-24883-1-git-send-email-nathan@nathan7.eu> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=001a11461710e30ac5052cec44dd X-Trace: ger.gmane.org 1456768186 22831 80.91.229.3 (29 Feb 2016 17:49:46 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 29 Feb 2016 17:49:46 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-9419-gllmg-musl=m.gmane.org@lists.openwall.com Mon Feb 29 18:49:39 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1aaRww-0001QL-65 for gllmg-musl@m.gmane.org; Mon, 29 Feb 2016 18:49:38 +0100 Original-Received: (qmail 32525 invoked by uid 550); 29 Feb 2016 17:49:36 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 32497 invoked from network); 29 Feb 2016 17:49:35 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nathan7.eu; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=RD5Cpkig6pJtTah/AAQPji+hlssKinHpC+SgteqavD8=; b=D0NQp30wCI+5YMrbjZ0ov6lv5JIesDx/1tpaD2o/vzVJyG2xVJeaODQKVRmkRI4RqX yWTCR8b7pz+8J8ihALu7U/gJckwZomSwqxyEEOnhVNk1RT1TJtG2p0SXjH46yZduy53R oPKjMzWzZ8fqCXEoSTlBSlI2GvUfXCaixMFIQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=RD5Cpkig6pJtTah/AAQPji+hlssKinHpC+SgteqavD8=; b=faywVx0pUUp/lx1727rQe8EKHlEDTgHFMvfMkmviBFUD+FCfGcM7uvXqv6JNkLkW05 egmNThjht1NhA2ihIdWlgy0Mtdaw4PzUoAC2mJKr4bKTqPjykKe/9b2TO7JHHsMi4Ow8 j3UgNj9B8ULwKbNLx5ipIhPEtsKJVhtA8b7IAACQoe9HTo1YF09lx5GxMhFtd6iWs8BG VTnG4Zyf/meneVhi87hfB76eNZz0gC1rT1HuaVmHz4IXtcBG2FAuNj8u8brBjGaDyFEp lIRAnqE2bEIV36ORwRSj6XhEegq1WWgs0ZVbjypCXstU1JDZOP3ip7mYZJ4wnC0Hgqap RdPg== X-Gm-Message-State: AD7BkJKlZXiDRhJfWD3QiBw5qLe4LAhPhBO8jFbKnHDitmc+2Uf3rfnyL87FqI9kH464qcN6ZiiUhUXAFMZNSg== X-Received: by 10.98.75.138 with SMTP id d10mr23937531pfj.108.1456768163181; Mon, 29 Feb 2016 09:49:23 -0800 (PST) In-Reply-To: Xref: news.gmane.org gmane.linux.lib.musl.general:9406 Archived-At: --001a11461710e30ac5052cec44dd Content-Type: text/plain; charset=UTF-8 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 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 > > Do you need this include? > > > +#include > > (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 > --001a11461710e30ac5052cec44dd Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
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, al= ong 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 tha= t
> otherwise has no libc wrapper.
>
> Adding it was discussed previously, without any objections:
> http://www.openwall.com/lists/musl/2015/05/08/2= 4
> ---
>=C2=A0 include/sched.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 3 +++<= br> >=C2=A0 src/sched/sched_getcpu.c | 10 ++++++++++
>=C2=A0 2 files changed, 13 insertions(+)
>=C2=A0 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 *);
>
>=C2=A0 typedef struct cpu_set_t { unsigned long __bits[128/sizeof(long)= ]; } cpu_set_t;
>=C2=A0 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
>=C2=A0 int sched_getaffinity(pid_t, size_t, cpu_set_t *);
>=C2=A0 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 wh= ether
such includes are desirable or not, so please wait for comment from Rich)
> +#include "syscall.h"
> +
> +int sched_getcpu(void) {
> +=C2=A0 int c, s;
> +=C2=A0 s =3D syscall(SYS_getcpu, &c, NULL, NULL);
> +=C2=A0 return (s =3D=3D 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 mayb= e
'return __syscall_ret(s ? s : c);'.

Alexander
--001a11461710e30ac5052cec44dd--