* Re: [PATCH v6 04/16] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-01-21 15:54 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190121151717.GK27931@hirez.programming.kicks-ass.net>
On 21-Jan 16:17, Peter Zijlstra wrote:
> On Tue, Jan 15, 2019 at 10:15:01AM +0000, Patrick Bellasi wrote:
> > +#ifdef CONFIG_UCLAMP_TASK
>
> > +struct uclamp_bucket {
> > + unsigned long value : bits_per(SCHED_CAPACITY_SCALE);
> > + unsigned long tasks : BITS_PER_LONG - bits_per(SCHED_CAPACITY_SCALE);
> > +};
>
> > +struct uclamp_cpu {
> > + unsigned int value;
>
> /* 4 byte hole */
>
> > + struct uclamp_bucket bucket[UCLAMP_BUCKETS];
> > +};
>
> With the default of 5, this UCLAMP_BUCKETS := 6, so struct uclamp_cpu
> ends up being 7 'unsigned long's, or 56 bytes on 64bit (with a 4 byte
> hole).
Yes, that's dimensioned and configured to fit into a single cache line
for all the possible 5 (by default) clamp values of a clamp index
(i.e. min or max util).
>
> > +#endif /* CONFIG_UCLAMP_TASK */
> > +
> > /*
> > * This is the main, per-CPU runqueue data structure.
> > *
> > @@ -835,6 +879,11 @@ struct rq {
> > unsigned long nr_load_updates;
> > u64 nr_switches;
> >
> > +#ifdef CONFIG_UCLAMP_TASK
> > + /* Utilization clamp values based on CPU's RUNNABLE tasks */
> > + struct uclamp_cpu uclamp[UCLAMP_CNT] ____cacheline_aligned;
>
> Which makes this 112 bytes with 8 bytes in 2 holes, which is short of 2
> 64 byte cachelines.
Right, we have 2 cache lines where:
- the first $L tracks 5 different util_min values
- the second $L tracks 5 different util_max values
> Is that the best layout?
It changed few times and that's what I found more reasonable for both
for fitting the default configuration and also for code readability.
Notice that we access RQ and SE clamp values with the same patter,
for example:
{rq|p}->uclamp[clamp_idx].value
Are you worried about the holes or something else specific ?
> > +#endif
> > +
> > struct cfs_rq cfs;
> > struct rt_rq rt;
> > struct dl_rq dl;
> > --
> > 2.19.2
> >
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: let filldir_t return bool instead of an error code
From: Jann Horn @ 2019-01-21 15:49 UTC (permalink / raw)
To: Dave Chinner
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Alexander Viro,
linux-fsdevel, Arnd Bergmann, Eric W. Biederman,
Theodore Ts'o, Andreas Dilger, linux-alpha, kernel list,
Pavel Machek, linux-arch, Linux API
In-Reply-To: <20190120224059.GA4205@dastard>
On Sun, Jan 20, 2019 at 11:41 PM Dave Chinner <david@fromorbit.com> wrote:
> On Fri, Jan 18, 2019 at 05:14:40PM +0100, Jann Horn wrote:
> > As Al Viro pointed out, many filldir_t functions return error codes, but
> > all callers of filldir_t functions just check whether the return value is
> > non-zero (to determine whether to continue reading the directory); more
> > precise errors have to be signalled via struct dir_context.
> > Change all filldir_t functions to return bool instead of int.
> >
> > Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> > arch/alpha/kernel/osf_sys.c | 12 +++----
> > fs/afs/dir.c | 30 +++++++++--------
> > fs/ecryptfs/file.c | 13 ++++----
> > fs/exportfs/expfs.c | 8 ++---
> > fs/fat/dir.c | 8 ++---
> > fs/gfs2/export.c | 6 ++--
> > fs/nfsd/nfs4recover.c | 8 ++---
> > fs/nfsd/vfs.c | 6 ++--
> > fs/ocfs2/dir.c | 10 +++---
> > fs/ocfs2/journal.c | 14 ++++----
> > fs/overlayfs/readdir.c | 24 +++++++-------
> > fs/readdir.c | 64 ++++++++++++++++++-------------------
> > fs/reiserfs/xattr.c | 20 ++++++------
> > fs/xfs/scrub/dir.c | 8 ++---
> > fs/xfs/scrub/parent.c | 4 +--
> > include/linux/fs.h | 10 +++---
> > 16 files changed, 125 insertions(+), 120 deletions(-)
> >
> > diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
> > index db1c2144d477..14e5ae0dac50 100644
> > --- a/arch/alpha/kernel/osf_sys.c
> > +++ b/arch/alpha/kernel/osf_sys.c
> > @@ -108,7 +108,7 @@ struct osf_dirent_callback {
> > int error;
> > };
> >
> > -static int
> > +static bool
> > osf_filldir(struct dir_context *ctx, const char *name, int namlen,
> > loff_t offset, u64 ino, unsigned int d_type)
> > {
> > @@ -120,14 +120,14 @@ osf_filldir(struct dir_context *ctx, const char *name, int namlen,
> >
> > buf->error = check_dirent_name(name, namlen);
> > if (unlikely(buf->error))
> > - return -EFSCORRUPTED;
> > + return false;
> > buf->error = -EINVAL; /* only used if we fail */
> > if (reclen > buf->count)
> > - return -EINVAL;
> > + return false;
>
> Oh, it's because the error being returned is being squashed by
> dir_emit():
Yeah.
> > struct dir_context {
> > @@ -3469,17 +3471,17 @@ static inline bool dir_emit(struct dir_context *ctx,
> > const char *name, int namelen,
> > u64 ino, unsigned type)
> > {
> > - return ctx->actor(ctx, name, namelen, ctx->pos, ino, type) == 0;
> > + return ctx->actor(ctx, name, namelen, ctx->pos, ino, type);
> > }
>
> /me wonders if it would be cleaner to do:
>
> static inline bool dir_emit(...)
> {
> buf->error = ctx->actor(....)
> if (buf->error)
> return false;
> return true;
> }
>
> And clean up all filldir actors just to return the error state
> rather than have to jump through hoops to stash the error state in
> the context buffer and return the error state?
One negative thing about that, IMO, is that it mixes up the request
for termination of the loop and the presence of an error. The current
code returns fake errors that never reach userspace in various places
to terminate the loop, e.g. fillonedir() has a "return -EINVAL" to
terminate the loop if at least one element has been read already, and
filldir() has a "return -EINTR" on signal_pending() that explicitly
only happens if at least one element has been read already.
But really, I don't have strong feelings about this, I just want to
get the "fs: don't let getdents return bogus names" patch in. :P
> That then allows callers who want/need the full error info can
> continue to call ctx->actor directly,
"continue to call ctx->actor directly"? I don't remember any code that
calls ctx->actor directly.
> while all those that just want
> to terminate their loops on error can continue just to call
> dir_emit()...
^ permalink raw reply
* Re: [PATCH v6 05/16] sched/core: uclamp: Update CPU's refcount on clamp changes
From: Patrick Bellasi @ 2019-01-21 15:44 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190121153308.GL27931@hirez.programming.kicks-ass.net>
On 21-Jan 16:33, Peter Zijlstra wrote:
> On Tue, Jan 15, 2019 at 10:15:02AM +0000, Patrick Bellasi wrote:
>
> > +static inline void
> > +uclamp_task_update_active(struct task_struct *p, unsigned int clamp_id)
> > +{
> > + struct rq_flags rf;
> > + struct rq *rq;
> > +
> > + /*
> > + * Lock the task and the CPU where the task is (or was) queued.
> > + *
> > + * We might lock the (previous) rq of a !RUNNABLE task, but that's the
> > + * price to pay to safely serialize util_{min,max} updates with
> > + * enqueues, dequeues and migration operations.
> > + * This is the same locking schema used by __set_cpus_allowed_ptr().
> > + */
> > + rq = task_rq_lock(p, &rf);
> > +
> > + /*
> > + * Setting the clamp bucket is serialized by task_rq_lock().
> > + * If the task is not yet RUNNABLE and its task_struct is not
> > + * affecting a valid clamp bucket, the next time it's enqueued,
> > + * it will already see the updated clamp bucket value.
> > + */
> > + if (!p->uclamp[clamp_id].active)
> > + goto done;
> > +
> > + uclamp_cpu_dec_id(p, rq, clamp_id);
> > + uclamp_cpu_inc_id(p, rq, clamp_id);
> > +
> > +done:
> > + task_rq_unlock(rq, p, &rf);
> > +}
>
> > @@ -1008,11 +1043,11 @@ static int __setscheduler_uclamp(struct task_struct *p,
> >
> > mutex_lock(&uclamp_mutex);
> > if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) {
> > - uclamp_bucket_inc(&p->uclamp[UCLAMP_MIN],
> > + uclamp_bucket_inc(p, &p->uclamp[UCLAMP_MIN],
> > UCLAMP_MIN, lower_bound);
> > }
> > if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MAX) {
> > - uclamp_bucket_inc(&p->uclamp[UCLAMP_MAX],
> > + uclamp_bucket_inc(p, &p->uclamp[UCLAMP_MAX],
> > UCLAMP_MAX, upper_bound);
> > }
> > mutex_unlock(&uclamp_mutex);
>
>
> But.... __sched_setscheduler() actually does the whole dequeue + enqueue
> thing already ?!? See where it does __setscheduler().
This is slow-path accounting, not fast path.
There are two refcounting going on here:
1) mapped buckets:
clamp_value <--(M1)--> bucket_id
2) RUNNABLE tasks:
bucket_id <--(M2)--> RUNNABLE tasks in a bucket
What we fix here is the refcounting for the buckets mapping. If a task
does not have a task specific clamp value it does not refcount any
bucket. The moment we assign a task specific clamp value, we need to
refcount the task in the bucket corresponding to that clamp value.
This will keep the bucket in use at least as long as the task will
need that clamp value.
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v6 03/16] sched/core: uclamp: Map TASK's clamp values into CPU's clamp buckets
From: Patrick Bellasi @ 2019-01-21 15:34 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190121150507.GJ27931@hirez.programming.kicks-ass.net>
On 21-Jan 16:05, Peter Zijlstra wrote:
> On Tue, Jan 15, 2019 at 10:15:00AM +0000, Patrick Bellasi wrote:
> > +static inline unsigned int uclamp_bucket_value(unsigned int clamp_value)
> > +{
> > +#define UCLAMP_BUCKET_DELTA (SCHED_CAPACITY_SCALE / CONFIG_UCLAMP_BUCKETS_COUNT)
> > +#define UCLAMP_BUCKET_UPPER (UCLAMP_BUCKET_DELTA * CONFIG_UCLAMP_BUCKETS_COUNT)
> > +
> > + if (clamp_value >= UCLAMP_BUCKET_UPPER)
> > + return SCHED_CAPACITY_SCALE;
> > +
> > + return UCLAMP_BUCKET_DELTA * (clamp_value / UCLAMP_BUCKET_DELTA);
> > +}
>
> > +static void uclamp_bucket_inc(struct uclamp_se *uc_se, unsigned int clamp_id,
> > + unsigned int clamp_value)
> > +{
> > + union uclamp_map *uc_maps = &uclamp_maps[clamp_id][0];
> > + unsigned int prev_bucket_id = uc_se->bucket_id;
> > + union uclamp_map uc_map_old, uc_map_new;
> > + unsigned int free_bucket_id;
> > + unsigned int bucket_value;
> > + unsigned int bucket_id;
> > +
> > + bucket_value = uclamp_bucket_value(clamp_value);
>
> Aahh!!
>
> So why don't you do:
>
> bucket_id = clamp_value / UCLAMP_BUCKET_DELTA;
> bucket_value = bucket_id * UCLAMP_BUCKET_DELTA;
The mapping done here is meant to keep at the beginning of the cache
line all and only the buckets we use. Let say we have configured the
system to track 20 buckets, to have a 5% clamping resolution, but then
we use only two values at run-time, e.g. 13% and 87%.
With the mapping done here the per-CPU variables will have to consider
only 2 buckets:
bucket_#00: clamp value: 10% (mapped)
bucket_#01: clamp value: 85% (mapped)
bucket_#02: (free)
...
bucket_#20: (free)
While without the mapping we will have:
bucket_#00: (free)
bucket_#01: clamp value: 10 (mapped)
bucket_#02: (free)
... big hole crossing a cache line ....
bucket_#16: (free)
bucket_#17: clamp value: 85 (mapped)
bucket_#18: (free)
...
bucket_#20: (free)
Addressing is simple without mapping but we can have performance
issues in the hot-path, since sometimes we need to scan all the
buckets to figure out the new max.
The mapping done here is meant to keep all the used slots at the very
beginning of a cache line to speed up that max computation when
required.
>
> > + do {
> > + /* Find the bucket_id of an already mapped clamp bucket... */
> > + free_bucket_id = UCLAMP_BUCKETS;
> > + for (bucket_id = 0; bucket_id < UCLAMP_BUCKETS; ++bucket_id) {
> > + uc_map_old.data = atomic_long_read(&uc_maps[bucket_id].adata);
> > + if (free_bucket_id == UCLAMP_BUCKETS && !uc_map_old.se_count)
> > + free_bucket_id = bucket_id;
> > + if (uc_map_old.value == bucket_value)
> > + break;
> > + }
> > +
> > + /* ... or allocate a new clamp bucket */
> > + if (bucket_id >= UCLAMP_BUCKETS) {
> > + /*
> > + * A valid clamp bucket must always be available.
> > + * If we cannot find one: refcounting is broken and we
> > + * warn once. The sched_entity will be tracked in the
> > + * fast-path using its previous clamp bucket, or not
> > + * tracked at all if not yet mapped (i.e. it's new).
> > + */
> > + if (unlikely(free_bucket_id == UCLAMP_BUCKETS)) {
> > + SCHED_WARN_ON(free_bucket_id == UCLAMP_BUCKETS);
> > + return;
> > + }
> > + bucket_id = free_bucket_id;
> > + uc_map_old.data = atomic_long_read(&uc_maps[bucket_id].adata);
> > + }
>
> And then skip all this?
> > +
> > + uc_map_new.se_count = uc_map_old.se_count + 1;
> > + uc_map_new.value = bucket_value;
> > +
> > + } while (!atomic_long_try_cmpxchg(&uc_maps[bucket_id].adata,
> > + &uc_map_old.data, uc_map_new.data));
> > +
> > + uc_se->value = clamp_value;
> > + uc_se->bucket_id = bucket_id;
> > +
> > + if (uc_se->mapped)
> > + uclamp_bucket_dec(clamp_id, prev_bucket_id);
> > +
> > + /*
> > + * Task's sched_entity are refcounted in the fast-path only when they
> > + * have got a valid clamp_bucket assigned.
> > + */
> > + uc_se->mapped = true;
> > +}
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v6 05/16] sched/core: uclamp: Update CPU's refcount on clamp changes
From: Peter Zijlstra @ 2019-01-21 15:33 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190115101513.2822-6-patrick.bellasi@arm.com>
On Tue, Jan 15, 2019 at 10:15:02AM +0000, Patrick Bellasi wrote:
> +static inline void
> +uclamp_task_update_active(struct task_struct *p, unsigned int clamp_id)
> +{
> + struct rq_flags rf;
> + struct rq *rq;
> +
> + /*
> + * Lock the task and the CPU where the task is (or was) queued.
> + *
> + * We might lock the (previous) rq of a !RUNNABLE task, but that's the
> + * price to pay to safely serialize util_{min,max} updates with
> + * enqueues, dequeues and migration operations.
> + * This is the same locking schema used by __set_cpus_allowed_ptr().
> + */
> + rq = task_rq_lock(p, &rf);
> +
> + /*
> + * Setting the clamp bucket is serialized by task_rq_lock().
> + * If the task is not yet RUNNABLE and its task_struct is not
> + * affecting a valid clamp bucket, the next time it's enqueued,
> + * it will already see the updated clamp bucket value.
> + */
> + if (!p->uclamp[clamp_id].active)
> + goto done;
> +
> + uclamp_cpu_dec_id(p, rq, clamp_id);
> + uclamp_cpu_inc_id(p, rq, clamp_id);
> +
> +done:
> + task_rq_unlock(rq, p, &rf);
> +}
> @@ -1008,11 +1043,11 @@ static int __setscheduler_uclamp(struct task_struct *p,
>
> mutex_lock(&uclamp_mutex);
> if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) {
> - uclamp_bucket_inc(&p->uclamp[UCLAMP_MIN],
> + uclamp_bucket_inc(p, &p->uclamp[UCLAMP_MIN],
> UCLAMP_MIN, lower_bound);
> }
> if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MAX) {
> - uclamp_bucket_inc(&p->uclamp[UCLAMP_MAX],
> + uclamp_bucket_inc(p, &p->uclamp[UCLAMP_MAX],
> UCLAMP_MAX, upper_bound);
> }
> mutex_unlock(&uclamp_mutex);
But.... __sched_setscheduler() actually does the whole dequeue + enqueue
thing already ?!? See where it does __setscheduler().
^ permalink raw reply
* Re: [PATCH v6 04/16] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-01-21 15:23 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190121145929.GI27931@hirez.programming.kicks-ass.net>
On 21-Jan 15:59, Peter Zijlstra wrote:
> On Tue, Jan 15, 2019 at 10:15:01AM +0000, Patrick Bellasi wrote:
> > @@ -835,6 +954,28 @@ static void uclamp_bucket_inc(struct uclamp_se *uc_se, unsigned int clamp_id,
> > } while (!atomic_long_try_cmpxchg(&uc_maps[bucket_id].adata,
> > &uc_map_old.data, uc_map_new.data));
> >
> > + /*
> > + * Ensure each CPU tracks the correct value for this clamp bucket.
> > + * This initialization of per-CPU variables is required only when a
> > + * clamp value is requested for the first time from a slow-path.
> > + */
>
> I'm confused; why is this needed?
That's a lazy initialization of the per-CPU uclamp data for a given
bucket, i.e. the clamp value assigned to a bucket, which happens only
when new clamp values are requested... usually only at system
boot/configuration time.
For example, let say we have these buckets mapped to given clamp
values:
bucket_#0: clamp value: 10% (mapped)
bucket_#1: clamp value: 20% (mapped)
bucket_#2: clamp value: 30% (mapped)
and then let's assume all the users of bucket_#1 are "destroyed", i.e.
there are no more tasks, system defaults or cgroups asking for a
20% clamp value. The corresponding bucket will become free:
bucket_#0: clamp value: 10% (mapped)
bucket_#1: clamp value: 20% (free)
bucket_#2: clamp value: 30% (mapped)
If, in the future, we ask for a new clamp value, let say a task ask
for a 40% clamp value, then we need to map that value into a bucket.
Since bucket_#1 is free we can use it to fill up the hold and keep all
the buckets in use at the beginning of a cache line.
However, since now bucket_#1 tracks a different clamp value (40
instead of 20) we need to walk all the CPUs and updated the cached
value:
bucket_#0: clamp value: 10% (mapped)
bucket_#1: clamp value: 40% (mapped)
bucket_#2: clamp value: 30% (mapped)
Is that more clear ?
In the following code:
>
> > + if (unlikely(!uc_map_old.se_count)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This condition is matched by clamp buckets which needs the
initialization described above. These are buckets without a client so
fare and that have been selected to map/track a new clamp value.
That's why we have an unlikely... quite likely tasks/cgroups will keep
asking for the same (limited number of) clamp values and thus we find
a bucket already properly initialized for them.
> > + for_each_possible_cpu(cpu) {
> > + struct uclamp_cpu *uc_cpu =
> > + &cpu_rq(cpu)->uclamp[clamp_id];
> > +
> > + /* CPU's tasks count must be 0 for free buckets */
> > + SCHED_WARN_ON(uc_cpu->bucket[bucket_id].tasks);
> > + if (unlikely(uc_cpu->bucket[bucket_id].tasks))
> > + uc_cpu->bucket[bucket_id].tasks = 0;
That's a safety check, we expect that (free) buckets do not refcount
any task. That's one of the conditions for a bucket to be considered
free. Here we do just a sanity check, that's because we use unlikely.
If the check matches there is a data corruption, which is reported by
the previous SCHED_WARN_ON and "fixed" by the if branch.
In my tests I have s/SCHED_WARN_ON/BUG_ON/ and never hit that bug...
thus the refcounting code should be ok and this check is there just to
be more on the safe side for future changes.
> > +
> > + /* Minimize cache lines invalidation */
> > + if (uc_cpu->bucket[bucket_id].value == bucket_value)
> > + continue;
If by any chance we get a request for a new clamp value which happened
to be already used before... we can skip the initialization to avoid.
> > + uc_cpu->bucket[bucket_id].value = bucket_value;
> > + }
> > + }
> > +
> > uc_se->value = clamp_value;
> > uc_se->bucket_id = bucket_id;
> >
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v6 04/16] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Peter Zijlstra @ 2019-01-21 15:17 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190115101513.2822-5-patrick.bellasi@arm.com>
On Tue, Jan 15, 2019 at 10:15:01AM +0000, Patrick Bellasi wrote:
> +#ifdef CONFIG_UCLAMP_TASK
> +struct uclamp_bucket {
> + unsigned long value : bits_per(SCHED_CAPACITY_SCALE);
> + unsigned long tasks : BITS_PER_LONG - bits_per(SCHED_CAPACITY_SCALE);
> +};
> +struct uclamp_cpu {
> + unsigned int value;
/* 4 byte hole */
> + struct uclamp_bucket bucket[UCLAMP_BUCKETS];
> +};
With the default of 5, this UCLAMP_BUCKETS := 6, so struct uclamp_cpu
ends up being 7 'unsigned long's, or 56 bytes on 64bit (with a 4 byte
hole).
> +#endif /* CONFIG_UCLAMP_TASK */
> +
> /*
> * This is the main, per-CPU runqueue data structure.
> *
> @@ -835,6 +879,11 @@ struct rq {
> unsigned long nr_load_updates;
> u64 nr_switches;
>
> +#ifdef CONFIG_UCLAMP_TASK
> + /* Utilization clamp values based on CPU's RUNNABLE tasks */
> + struct uclamp_cpu uclamp[UCLAMP_CNT] ____cacheline_aligned;
Which makes this 112 bytes with 8 bytes in 2 holes, which is short of 2
64 byte cachelines.
Is that the best layout?
> +#endif
> +
> struct cfs_rq cfs;
> struct rt_rq rt;
> struct dl_rq dl;
> --
> 2.19.2
>
^ permalink raw reply
* Re: [PATCH v6 03/16] sched/core: uclamp: Map TASK's clamp values into CPU's clamp buckets
From: Peter Zijlstra @ 2019-01-21 15:05 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190115101513.2822-4-patrick.bellasi@arm.com>
On Tue, Jan 15, 2019 at 10:15:00AM +0000, Patrick Bellasi wrote:
> +static inline unsigned int uclamp_bucket_value(unsigned int clamp_value)
> +{
> +#define UCLAMP_BUCKET_DELTA (SCHED_CAPACITY_SCALE / CONFIG_UCLAMP_BUCKETS_COUNT)
> +#define UCLAMP_BUCKET_UPPER (UCLAMP_BUCKET_DELTA * CONFIG_UCLAMP_BUCKETS_COUNT)
> +
> + if (clamp_value >= UCLAMP_BUCKET_UPPER)
> + return SCHED_CAPACITY_SCALE;
> +
> + return UCLAMP_BUCKET_DELTA * (clamp_value / UCLAMP_BUCKET_DELTA);
> +}
> +static void uclamp_bucket_inc(struct uclamp_se *uc_se, unsigned int clamp_id,
> + unsigned int clamp_value)
> +{
> + union uclamp_map *uc_maps = &uclamp_maps[clamp_id][0];
> + unsigned int prev_bucket_id = uc_se->bucket_id;
> + union uclamp_map uc_map_old, uc_map_new;
> + unsigned int free_bucket_id;
> + unsigned int bucket_value;
> + unsigned int bucket_id;
> +
> + bucket_value = uclamp_bucket_value(clamp_value);
Aahh!!
So why don't you do:
bucket_id = clamp_value / UCLAMP_BUCKET_DELTA;
bucket_value = bucket_id * UCLAMP_BUCKET_DELTA;
> + do {
> + /* Find the bucket_id of an already mapped clamp bucket... */
> + free_bucket_id = UCLAMP_BUCKETS;
> + for (bucket_id = 0; bucket_id < UCLAMP_BUCKETS; ++bucket_id) {
> + uc_map_old.data = atomic_long_read(&uc_maps[bucket_id].adata);
> + if (free_bucket_id == UCLAMP_BUCKETS && !uc_map_old.se_count)
> + free_bucket_id = bucket_id;
> + if (uc_map_old.value == bucket_value)
> + break;
> + }
> +
> + /* ... or allocate a new clamp bucket */
> + if (bucket_id >= UCLAMP_BUCKETS) {
> + /*
> + * A valid clamp bucket must always be available.
> + * If we cannot find one: refcounting is broken and we
> + * warn once. The sched_entity will be tracked in the
> + * fast-path using its previous clamp bucket, or not
> + * tracked at all if not yet mapped (i.e. it's new).
> + */
> + if (unlikely(free_bucket_id == UCLAMP_BUCKETS)) {
> + SCHED_WARN_ON(free_bucket_id == UCLAMP_BUCKETS);
> + return;
> + }
> + bucket_id = free_bucket_id;
> + uc_map_old.data = atomic_long_read(&uc_maps[bucket_id].adata);
> + }
And then skip all this?
> +
> + uc_map_new.se_count = uc_map_old.se_count + 1;
> + uc_map_new.value = bucket_value;
> +
> + } while (!atomic_long_try_cmpxchg(&uc_maps[bucket_id].adata,
> + &uc_map_old.data, uc_map_new.data));
> +
> + uc_se->value = clamp_value;
> + uc_se->bucket_id = bucket_id;
> +
> + if (uc_se->mapped)
> + uclamp_bucket_dec(clamp_id, prev_bucket_id);
> +
> + /*
> + * Task's sched_entity are refcounted in the fast-path only when they
> + * have got a valid clamp_bucket assigned.
> + */
> + uc_se->mapped = true;
> +}
^ permalink raw reply
* Re: [PATCH v6 04/16] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Peter Zijlstra @ 2019-01-21 14:59 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190115101513.2822-5-patrick.bellasi@arm.com>
On Tue, Jan 15, 2019 at 10:15:01AM +0000, Patrick Bellasi wrote:
> @@ -835,6 +954,28 @@ static void uclamp_bucket_inc(struct uclamp_se *uc_se, unsigned int clamp_id,
> } while (!atomic_long_try_cmpxchg(&uc_maps[bucket_id].adata,
> &uc_map_old.data, uc_map_new.data));
>
> + /*
> + * Ensure each CPU tracks the correct value for this clamp bucket.
> + * This initialization of per-CPU variables is required only when a
> + * clamp value is requested for the first time from a slow-path.
> + */
I'm confused; why is this needed?
> + if (unlikely(!uc_map_old.se_count)) {
> + for_each_possible_cpu(cpu) {
> + struct uclamp_cpu *uc_cpu =
> + &cpu_rq(cpu)->uclamp[clamp_id];
> +
> + /* CPU's tasks count must be 0 for free buckets */
> + SCHED_WARN_ON(uc_cpu->bucket[bucket_id].tasks);
> + if (unlikely(uc_cpu->bucket[bucket_id].tasks))
> + uc_cpu->bucket[bucket_id].tasks = 0;
> +
> + /* Minimize cache lines invalidation */
> + if (uc_cpu->bucket[bucket_id].value == bucket_value)
> + continue;
> + uc_cpu->bucket[bucket_id].value = bucket_value;
> + }
> + }
> +
> uc_se->value = clamp_value;
> uc_se->bucket_id = bucket_id;
>
^ permalink raw reply
* Re: [PATCH v6 03/16] sched/core: uclamp: Map TASK's clamp values into CPU's clamp buckets
From: Peter Zijlstra @ 2019-01-21 12:51 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190121122659.if6f4ubw2wgblxwi@e110439-lin>
On Mon, Jan 21, 2019 at 12:27:10PM +0000, Patrick Bellasi wrote:
> On 21-Jan 11:15, Peter Zijlstra wrote:
> > On Tue, Jan 15, 2019 at 10:15:00AM +0000, Patrick Bellasi wrote:
> > > +/*
> > > + * Number of utilization clamp buckets.
> > > + *
> > > + * The first clamp bucket (bucket_id=0) is used to track non clamped tasks, i.e.
> > > + * util_{min,max} (0,SCHED_CAPACITY_SCALE). Thus we allocate one more bucket in
> > > + * addition to the compile time configured number.
> > > + */
> > > +#define UCLAMP_BUCKETS (CONFIG_UCLAMP_BUCKETS_COUNT + 1)
> > > +
> > > +/*
> > > + * Utilization clamp bucket
> > > + * @value: clamp value tracked by a clamp bucket
> > > + * @bucket_id: the bucket index used by the fast-path
> > > + * @mapped: the bucket index is valid
> > > + *
> > > + * A utilization clamp bucket maps a:
> > > + * clamp value (value), i.e.
> > > + * util_{min,max} value requested from userspace
> > > + * to a:
> > > + * clamp bucket index (bucket_id), i.e.
> > > + * index of the per-cpu RUNNABLE tasks refcounting array
> > > + *
> > > + * The mapped bit is set whenever a task has been mapped on a clamp bucket for
> > > + * the first time. When this bit is set, any:
> > > + * uclamp_bucket_inc() - for a new clamp value
> > > + * is matched by a:
> > > + * uclamp_bucket_dec() - for the old clamp value
> > > + */
> > > +struct uclamp_se {
> > > + unsigned int value : bits_per(SCHED_CAPACITY_SCALE);
> > > + unsigned int bucket_id : bits_per(UCLAMP_BUCKETS);
> > > + unsigned int mapped : 1;
> > > +};
> >
> > Do we want something like:
> >
> > BUILD_BUG_ON(sizeof(struct uclamp_se) == sizeof(unsigned int));
>
> Mmm... isn't "!=" what you mean ?
Quite.
> We cannot use less then an unsigned int for the fields above... am I
> missing something?
I wanted to ensure we don't accidentally use more.
> > And/or put a limit on CONFIG_UCLAMP_BUCKETS_COUNT that guarantees that ?
>
> The number of buckets is currently KConfig limited to a max of 20, which gives:
>
> UCLAMP_BUCKETS: 21 => 5bits
>
> Thus, even on 32 bit targets and assuming 21bits for an "extended"
> SCHED_CAPACITY_SCALE range we should always fit into an unsigned int
> and have at least 6 bits for flags.
>
> Are you afraid of some compiler magic related to bitfields packing ?
Nah, I missed the Kconfig limit and was afraid that some weird configs
would end up with massively huge structures.
^ permalink raw reply
* Re: [PATCH v6 03/16] sched/core: uclamp: Map TASK's clamp values into CPU's clamp buckets
From: Patrick Bellasi @ 2019-01-21 12:27 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190121101508.GH27931@hirez.programming.kicks-ass.net>
On 21-Jan 11:15, Peter Zijlstra wrote:
> On Tue, Jan 15, 2019 at 10:15:00AM +0000, Patrick Bellasi wrote:
> > +/*
> > + * Number of utilization clamp buckets.
> > + *
> > + * The first clamp bucket (bucket_id=0) is used to track non clamped tasks, i.e.
> > + * util_{min,max} (0,SCHED_CAPACITY_SCALE). Thus we allocate one more bucket in
> > + * addition to the compile time configured number.
> > + */
> > +#define UCLAMP_BUCKETS (CONFIG_UCLAMP_BUCKETS_COUNT + 1)
> > +
> > +/*
> > + * Utilization clamp bucket
> > + * @value: clamp value tracked by a clamp bucket
> > + * @bucket_id: the bucket index used by the fast-path
> > + * @mapped: the bucket index is valid
> > + *
> > + * A utilization clamp bucket maps a:
> > + * clamp value (value), i.e.
> > + * util_{min,max} value requested from userspace
> > + * to a:
> > + * clamp bucket index (bucket_id), i.e.
> > + * index of the per-cpu RUNNABLE tasks refcounting array
> > + *
> > + * The mapped bit is set whenever a task has been mapped on a clamp bucket for
> > + * the first time. When this bit is set, any:
> > + * uclamp_bucket_inc() - for a new clamp value
> > + * is matched by a:
> > + * uclamp_bucket_dec() - for the old clamp value
> > + */
> > +struct uclamp_se {
> > + unsigned int value : bits_per(SCHED_CAPACITY_SCALE);
> > + unsigned int bucket_id : bits_per(UCLAMP_BUCKETS);
> > + unsigned int mapped : 1;
> > +};
>
> Do we want something like:
>
> BUILD_BUG_ON(sizeof(struct uclamp_se) == sizeof(unsigned int));
Mmm... isn't "!=" what you mean ?
We cannot use less then an unsigned int for the fields above... am I
missing something?
> And/or put a limit on CONFIG_UCLAMP_BUCKETS_COUNT that guarantees that ?
The number of buckets is currently KConfig limited to a max of 20, which gives:
UCLAMP_BUCKETS: 21 => 5bits
Thus, even on 32 bit targets and assuming 21bits for an "extended"
SCHED_CAPACITY_SCALE range we should always fit into an unsigned int
and have at least 6 bits for flags.
Are you afraid of some compiler magic related to bitfields packing ?
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v2 29/29] y2038: add 64-bit time_t syscalls to all 32-bit architectures
From: Heiko Carstens @ 2019-01-21 12:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038, linux-api, linux-kernel, linux-arch, mattst88, linux,
catalin.marinas, will.deacon, tony.luck, fenghua.yu, geert,
monstr, paul.burton, deller, benh, mpe, schwidefsky, dalias,
davem, luto, tglx, mingo, hpa, x86, jcmvbkbc, akpm, deepa.kernel,
ebiederm, firoz.khan, linux-alpha, linux-arm-kernel, linux-ia64,
linux-m68k, linux-mips, linux-p
In-Reply-To: <20190118161835.2259170-30-arnd@arndb.de>
On Fri, Jan 18, 2019 at 05:18:35PM +0100, Arnd Bergmann wrote:
> This adds 21 new system calls on each ABI that has 32-bit time_t
> today. All of these have the exact same semantics as their existing
> counterparts, and the new ones all have macro names that end in 'time64'
> for clarification.
>
> This gets us to the point of being able to safely use a C library
> that has 64-bit time_t in user space. There are still a couple of
> loose ends to tie up in various areas of the code, but this is the
> big one, and should be entirely uncontroversial at this point.
>
> In particular, there are four system calls (getitimer, setitimer,
> waitid, and getrusage) that don't have a 64-bit counterpart yet,
> but these can all be safely implemented in the C library by wrapping
> around the existing system calls because the 32-bit time_t they
> pass only counts elapsed time, not time since the epoch. They
> will be dealt with later.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/s390/kernel/syscalls/syscall.tbl | 20 +++++++++
For the s390 bits:
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
^ permalink raw reply
* Re: [PATCH v2 28/29] y2038: rename old time and utime syscalls
From: Heiko Carstens @ 2019-01-21 12:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038, linux-api, linux-kernel, linux-arch, mattst88, linux,
catalin.marinas, will.deacon, tony.luck, fenghua.yu, geert,
monstr, paul.burton, deller, benh, mpe, schwidefsky, dalias,
davem, luto, tglx, mingo, hpa, x86, jcmvbkbc, akpm, deepa.kernel,
ebiederm, firoz.khan, linux-alpha, linux-arm-kernel, linux-ia64,
linux-m68k, linux-mips, linux-p
In-Reply-To: <20190118161835.2259170-29-arnd@arndb.de>
On Fri, Jan 18, 2019 at 05:18:34PM +0100, Arnd Bergmann wrote:
> The time, stime, utime, utimes, and futimesat system calls are only
> used on older architectures, and we do not provide y2038 safe variants
> of them, as they are replaced by clock_gettime64, clock_settime64,
> and utimensat_time64.
>
> However, for consistency it seems better to have the 32-bit architectures
> that still use them call the "time32" entry points (leaving the
> traditional handlers for the 64-bit architectures), like we do for system
> calls that now require two versions.
>
> Note: We used to always define __ARCH_WANT_SYS_TIME and
> __ARCH_WANT_SYS_UTIME and only set __ARCH_WANT_COMPAT_SYS_TIME and
> __ARCH_WANT_SYS_UTIME32 for compat mode on 64-bit kernels. Now this is
> reversed: only 64-bit architectures set __ARCH_WANT_SYS_TIME/UTIME, while
> we need __ARCH_WANT_SYS_TIME32/UTIME32 for 32-bit architectures and compat
> mode. The resulting asm/unistd.h changes look a bit counterintuitive.
>
> This is only a cleanup patch and it should not change any behavior.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
...
> arch/s390/include/asm/unistd.h | 2 +-
For the s390 bits:
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
^ permalink raw reply
* Re: [PATCH v2 17/29] syscalls: remove obsolete __IGNORE_ macros
From: Heiko Carstens @ 2019-01-21 11:59 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038, linux-api, linux-kernel, linux-arch, mattst88, linux,
catalin.marinas, will.deacon, tony.luck, fenghua.yu, geert,
monstr, paul.burton, deller, benh, mpe, schwidefsky, dalias,
davem, luto, tglx, mingo, hpa, x86, jcmvbkbc, akpm, deepa.kernel,
ebiederm, firoz.khan, linux-alpha, linux-arm-kernel, linux-ia64,
linux-m68k, linux-mips, linux-p
In-Reply-To: <20190118161835.2259170-18-arnd@arndb.de>
On Fri, Jan 18, 2019 at 05:18:23PM +0100, Arnd Bergmann wrote:
> These are all for ignoring the lack of obsolete system calls,
> which have been marked the same way in scripts/checksyscall.sh,
> so these can be removed.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/mips/include/asm/unistd.h | 16 ----------------
> arch/parisc/include/asm/unistd.h | 3 ---
> arch/s390/include/asm/unistd.h | 2 --
> arch/xtensa/include/asm/unistd.h | 12 ------------
> 4 files changed, 33 deletions(-)
For the s390 bits:
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
^ permalink raw reply
* Re: [PATCH v2 14/29] arch: add pkey and rseq syscall numbers everywhere
From: Heiko Carstens @ 2019-01-21 11:59 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038, linux-api, linux-kernel, linux-arch, mattst88, linux,
catalin.marinas, will.deacon, tony.luck, fenghua.yu, geert,
monstr, paul.burton, deller, benh, mpe, schwidefsky, dalias,
davem, luto, tglx, mingo, hpa, x86, jcmvbkbc, akpm, deepa.kernel,
ebiederm, firoz.khan, linux-alpha, linux-arm-kernel, linux-ia64,
linux-m68k, linux-mips, linux-p
In-Reply-To: <20190118161835.2259170-15-arnd@arndb.de>
On Fri, Jan 18, 2019 at 05:18:20PM +0100, Arnd Bergmann wrote:
> Most architectures define system call numbers for the rseq and pkey system
> calls, even when they don't support the features, and perhaps never will.
>
> Only a few architectures are missing these, so just define them anyway
> for consistency. If we decide to add them later to one of these, the
> system call numbers won't get out of sync then.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/alpha/include/asm/unistd.h | 4 ----
> arch/alpha/kernel/syscalls/syscall.tbl | 4 ++++
> arch/ia64/kernel/syscalls/syscall.tbl | 4 ++++
> arch/m68k/kernel/syscalls/syscall.tbl | 4 ++++
> arch/parisc/include/asm/unistd.h | 3 ---
> arch/parisc/kernel/syscalls/syscall.tbl | 4 ++++
> arch/s390/include/asm/unistd.h | 3 ---
> arch/s390/kernel/syscalls/syscall.tbl | 3 +++
> arch/sh/kernel/syscalls/syscall.tbl | 4 ++++
> arch/sparc/include/asm/unistd.h | 5 -----
> arch/sparc/kernel/syscalls/syscall.tbl | 4 ++++
> arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
> 12 files changed, 28 insertions(+), 15 deletions(-)
For the s390 bits:
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
^ permalink raw reply
* Re: [PATCH v2 13/29] arch: add split IPC system calls where needed
From: Heiko Carstens @ 2019-01-21 11:57 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038, linux-api, linux-kernel, linux-arch, mattst88, linux,
catalin.marinas, will.deacon, tony.luck, fenghua.yu, geert,
monstr, paul.burton, deller, benh, mpe, schwidefsky, dalias,
davem, luto, tglx, mingo, hpa, x86, jcmvbkbc, akpm, deepa.kernel,
ebiederm, firoz.khan, linux-alpha, linux-arm-kernel, linux-ia64,
linux-m68k, linux-mips, linux-p
In-Reply-To: <20190118161835.2259170-14-arnd@arndb.de>
On Fri, Jan 18, 2019 at 05:18:19PM +0100, Arnd Bergmann wrote:
> The IPC system call handling is highly inconsistent across architectures,
> some use sys_ipc, some use separate calls, and some use both. We also
> have some architectures that require passing IPC_64 in the flags, and
> others that set it implicitly.
>
> For the additon of a y2083 safe semtimedop() system call, I chose to only
> support the separate entry points, but that requires first supporting
> the regular ones with their own syscall numbers.
>
> The IPC_64 is now implied by the new semctl/shmctl/msgctl system
> calls even on the architectures that require passing it with the ipc()
> multiplexer.
>
> I'm not adding the new semtimedop() or semop() on 32-bit architectures,
> those will get implemented using the new semtimedop_time64() version
> that gets added along with the other time64 calls.
> Three 64-bit architectures (powerpc, s390 and sparc) get semtimedop().
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> One aspect here that might be a bit controversial is the use of
> the same system call numbers across all architectures, synchronizing
> all of them with the x86-32 numbers. With the new syscall.tbl
> files, I hope we can just keep doing that in the future, and no
> longer require the architecture maintainers to assign a number.
>
> This is mainly useful for implementers of the C libraries: if
> we can add future system calls everywhere at the same time, using
> a particular version of the kernel headers also guarantees that
> the system call number macro is visible.
> ---
> arch/m68k/kernel/syscalls/syscall.tbl | 11 +++++++++++
> arch/mips/kernel/syscalls/syscall_o32.tbl | 11 +++++++++++
> arch/powerpc/kernel/syscalls/syscall.tbl | 13 +++++++++++++
> arch/s390/kernel/syscalls/syscall.tbl | 12 ++++++++++++
> arch/sh/kernel/syscalls/syscall.tbl | 11 +++++++++++
> arch/sparc/kernel/syscalls/syscall.tbl | 12 ++++++++++++
> arch/x86/entry/syscalls/syscall_32.tbl | 11 +++++++++++
> 7 files changed, 81 insertions(+)
For the s390 bits:
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
^ permalink raw reply
* Re: [PATCH v6 03/16] sched/core: uclamp: Map TASK's clamp values into CPU's clamp buckets
From: Peter Zijlstra @ 2019-01-21 10:15 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190115101513.2822-4-patrick.bellasi@arm.com>
On Tue, Jan 15, 2019 at 10:15:00AM +0000, Patrick Bellasi wrote:
> +/*
> + * Number of utilization clamp buckets.
> + *
> + * The first clamp bucket (bucket_id=0) is used to track non clamped tasks, i.e.
> + * util_{min,max} (0,SCHED_CAPACITY_SCALE). Thus we allocate one more bucket in
> + * addition to the compile time configured number.
> + */
> +#define UCLAMP_BUCKETS (CONFIG_UCLAMP_BUCKETS_COUNT + 1)
> +
> +/*
> + * Utilization clamp bucket
> + * @value: clamp value tracked by a clamp bucket
> + * @bucket_id: the bucket index used by the fast-path
> + * @mapped: the bucket index is valid
> + *
> + * A utilization clamp bucket maps a:
> + * clamp value (value), i.e.
> + * util_{min,max} value requested from userspace
> + * to a:
> + * clamp bucket index (bucket_id), i.e.
> + * index of the per-cpu RUNNABLE tasks refcounting array
> + *
> + * The mapped bit is set whenever a task has been mapped on a clamp bucket for
> + * the first time. When this bit is set, any:
> + * uclamp_bucket_inc() - for a new clamp value
> + * is matched by a:
> + * uclamp_bucket_dec() - for the old clamp value
> + */
> +struct uclamp_se {
> + unsigned int value : bits_per(SCHED_CAPACITY_SCALE);
> + unsigned int bucket_id : bits_per(UCLAMP_BUCKETS);
> + unsigned int mapped : 1;
> +};
Do we want something like:
BUILD_BUG_ON(sizeof(struct uclamp_se) == sizeof(unsigned int));
And/or put a limit on CONFIG_UCLAMP_BUCKETS_COUNT that guarantees that ?
^ permalink raw reply
* Re: [PATCH v2 29/29] y2038: add 64-bit time_t syscalls to all 32-bit architectures
From: Geert Uytterhoeven @ 2019-01-21 8:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038 Mailman List, Linux API, Linux Kernel Mailing List,
Linux-Arch, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Tony Luck, Fenghua Yu, Michal Simek, Paul Burton,
Helge Deller, Benjamin Herrenschmidt, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Rich Felker, David S. Miller
In-Reply-To: <20190118161835.2259170-30-arnd@arndb.de>
On Fri, Jan 18, 2019 at 5:25 PM Arnd Bergmann <arnd@arndb.de> wrote:
> This adds 21 new system calls on each ABI that has 32-bit time_t
> today. All of these have the exact same semantics as their existing
> counterparts, and the new ones all have macro names that end in 'time64'
> for clarification.
>
> This gets us to the point of being able to safely use a C library
> that has 64-bit time_t in user space. There are still a couple of
> loose ends to tie up in various areas of the code, but this is the
> big one, and should be entirely uncontroversial at this point.
>
> In particular, there are four system calls (getitimer, setitimer,
> waitid, and getrusage) that don't have a 64-bit counterpart yet,
> but these can all be safely implemented in the C library by wrapping
> around the existing system calls because the 32-bit time_t they
> pass only counts elapsed time, not time since the epoch. They
> will be dealt with later.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> arch/m68k/kernel/syscalls/syscall.tbl | 20 +++++++++
For m68k:
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 28/29] y2038: rename old time and utime syscalls
From: Geert Uytterhoeven @ 2019-01-21 8:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038 Mailman List, Linux API, Linux Kernel Mailing List,
Linux-Arch, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Tony Luck, Fenghua Yu, Michal Simek, Paul Burton,
Helge Deller, Benjamin Herrenschmidt, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Rich Felker, David S. Miller
In-Reply-To: <20190118161835.2259170-29-arnd@arndb.de>
On Fri, Jan 18, 2019 at 5:21 PM Arnd Bergmann <arnd@arndb.de> wrote:
> The time, stime, utime, utimes, and futimesat system calls are only
> used on older architectures, and we do not provide y2038 safe variants
> of them, as they are replaced by clock_gettime64, clock_settime64,
> and utimensat_time64.
>
> However, for consistency it seems better to have the 32-bit architectures
> that still use them call the "time32" entry points (leaving the
> traditional handlers for the 64-bit architectures), like we do for system
> calls that now require two versions.
>
> Note: We used to always define __ARCH_WANT_SYS_TIME and
> __ARCH_WANT_SYS_UTIME and only set __ARCH_WANT_COMPAT_SYS_TIME and
> __ARCH_WANT_SYS_UTIME32 for compat mode on 64-bit kernels. Now this is
> reversed: only 64-bit architectures set __ARCH_WANT_SYS_TIME/UTIME, while
> we need __ARCH_WANT_SYS_TIME32/UTIME32 for 32-bit architectures and compat
> mode. The resulting asm/unistd.h changes look a bit counterintuitive.
>
> This is only a cleanup patch and it should not change any behavior.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> arch/m68k/include/asm/unistd.h | 4 ++--
> arch/m68k/kernel/syscalls/syscall.tbl | 10 +++++-----
For m68k:
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: [PATCH v2 26/29] y2038: use time32 syscall names on 32-bit
From: Geert Uytterhoeven @ 2019-01-21 8:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038 Mailman List, Linux API, Linux Kernel Mailing List,
Linux-Arch, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Tony Luck, Fenghua Yu, Michal Simek, Paul Burton,
Helge Deller, Benjamin Herrenschmidt, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Rich Felker, David S. Miller
In-Reply-To: <20190118161835.2259170-27-arnd@arndb.de>
On Fri, Jan 18, 2019 at 5:21 PM Arnd Bergmann <arnd@arndb.de> wrote:
> This is the big flip, where all 32-bit architectures set COMPAT_32BIT_TIME
> abd use the _time32 system calls from the former compat layer instead
> of the system calls that take __kernel_timespec and similar arguments.
>
> The temporary redirects for __kernel_timespec, __kernel_itimerspec
> and __kernel_timex can get removed with this.
>
> It would be easy to split this commit by architecture, but with the new
> generated system call tables, it's easy enough to do it all at once,
> which makes it a little easier to check that the changes are the same
> in each table.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> arch/m68k/kernel/syscalls/syscall.tbl | 42 +++++------
For m68k:
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 13/29] arch: add split IPC system calls where needed
From: Geert Uytterhoeven @ 2019-01-21 8:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038 Mailman List, Linux API, Linux Kernel Mailing List,
Linux-Arch, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Tony Luck, Fenghua Yu, Michal Simek, Paul Burton,
Helge Deller, Benjamin Herrenschmidt, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Rich Felker, David S. Miller
In-Reply-To: <20190118161835.2259170-14-arnd@arndb.de>
On Fri, Jan 18, 2019 at 5:20 PM Arnd Bergmann <arnd@arndb.de> wrote:
> The IPC system call handling is highly inconsistent across architectures,
> some use sys_ipc, some use separate calls, and some use both. We also
> have some architectures that require passing IPC_64 in the flags, and
> others that set it implicitly.
>
> For the additon of a y2083 safe semtimedop() system call, I chose to only
> support the separate entry points, but that requires first supporting
> the regular ones with their own syscall numbers.
>
> The IPC_64 is now implied by the new semctl/shmctl/msgctl system
> calls even on the architectures that require passing it with the ipc()
> multiplexer.
>
> I'm not adding the new semtimedop() or semop() on 32-bit architectures,
> those will get implemented using the new semtimedop_time64() version
> that gets added along with the other time64 calls.
> Three 64-bit architectures (powerpc, s390 and sparc) get semtimedop().
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> arch/m68k/kernel/syscalls/syscall.tbl | 11 +++++++++++
For m68k:
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 14/29] arch: add pkey and rseq syscall numbers everywhere
From: Geert Uytterhoeven @ 2019-01-21 8:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038 Mailman List, Linux API, Linux Kernel Mailing List,
Linux-Arch, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Tony Luck, Fenghua Yu, Michal Simek, Paul Burton,
Helge Deller, Benjamin Herrenschmidt, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Rich Felker, David S. Miller
In-Reply-To: <20190118161835.2259170-15-arnd@arndb.de>
On Fri, Jan 18, 2019 at 5:20 PM Arnd Bergmann <arnd@arndb.de> wrote:
> Most architectures define system call numbers for the rseq and pkey system
> calls, even when they don't support the features, and perhaps never will.
>
> Only a few architectures are missing these, so just define them anyway
> for consistency. If we decide to add them later to one of these, the
> system call numbers won't get out of sync then.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> arch/m68k/kernel/syscalls/syscall.tbl | 4 ++++
> --- a/arch/m68k/kernel/syscalls/syscall.tbl
> +++ b/arch/m68k/kernel/syscalls/syscall.tbl
> @@ -388,6 +388,10 @@
> 378 common pwritev2 sys_pwritev2
> 379 common statx sys_statx
> 380 common seccomp sys_seccomp
> +381 common pkey_alloc sys_pkey_alloc
> +382 common pkey_free sys_pkey_free
> +383 common pkey_mprotect sys_pkey_mprotect
> +384 common rseq sys_rseq
Note that all architectures that already define pkey syscalls, list
pkey_mprotect
first.
Regardless, for m68k:
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> # room for arch specific calls
> 393 common semget sys_semget
> 394 common semctl sys_semctl
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 08/29] m68k: assign syscall number for seccomp
From: Geert Uytterhoeven @ 2019-01-21 8:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rich Felker, linux-ia64@vger.kernel.org, Linux-sh list,
Benjamin Herrenschmidt, Heiko Carstens, linux-mips, Max Filippov,
netdev, Deepa Dinamani, H. Peter Anvin, sparclinux, Linux-Arch,
linux-s390, y2038 Mailman List, Michael Ellerman, Helge Deller,
the arch/x86 maintainers, Russell King, Ingo Molnar, Firoz Khan,
Catalin Marinas, Matt Turner, Fenghua Yu <fenghua.>
In-Reply-To: <20190118161835.2259170-9-arnd@arndb.de>
On Fri, Jan 18, 2019 at 5:20 PM Arnd Bergmann <arnd@arndb.de> wrote:
> Most architectures have assigned a numbers for the seccomp syscall
> even when they do not implement it.
>
> m68k is an exception here, so for consistency lets add the number.
> Unless CONFIG_SECCOMP is implemented, the system call just
> returns -ENOSYS.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 29/29] y2038: add 64-bit time_t syscalls to all 32-bit architectures
From: Geert Uytterhoeven @ 2019-01-21 8:19 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Andy Lutomirski, Arnd Bergmann, y2038 Mailman List, Linux API,
LKML, linux-arch, Matt Turner, Catalin Marinas, Will Deacon,
Tony Luck, Fenghua Yu, Michal Simek, Paul Burton, Helge Deller,
Benjamin Herrenschmidt, Michael Ellerman, Martin Schwidefsky,
Heiko Carstens, Rich Felker, David S. Miller
In-Reply-To: <20190119142852.cntdihah4mpa3lgx@e5254000004ec.dyn.armlinux.org.uk>
Hi Russell,
On Sat, Jan 19, 2019 at 3:29 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
> On Fri, Jan 18, 2019 at 11:53:25AM -0800, Andy Lutomirski wrote:
> > On Fri, Jan 18, 2019 at 11:33 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Fri, Jan 18, 2019 at 7:50 PM Andy Lutomirski <luto@kernel.org> wrote:
> > > > On Fri, Jan 18, 2019 at 8:25 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > - Once we get to 512, we clash with the x32 numbers (unless
> > > > > we remove x32 support first), and probably have to skip
> > > > > a few more. I also considered using the 512..547 space
> > > > > for 32-bit-only calls (which never clash with x32), but
> > > > > that also seems to add a bit of complexity.
> > > >
> > > > I have a patch that I'll send soon to make x32 use its own table. As
> > > > far as I'm concerned, 547 is *it*. 548 is just a normal number and is
> > > > not special. But let's please not reuse 512..547 for other purposes
> > > > on x86 variants -- that way lies even more confusion, IMO.
> > >
> > > Fair enough, the space for those numbers is cheap enough here.
> > > I take it you mean we also should not reuse that number space if
> > > we were to decide to remove x32 soon, but you are not worried
> > > about clashing with arch/alpha when everything else uses consistent
> > > numbers?
> > >
> >
> > I think we have two issues if we reuse those numbers for new syscalls.
> > First, I'd really like to see new syscalls be numbered consistently
> > everywhere, or at least on all x86 variants, and we can't on x32
> > because they mean something else. Perhaps more importantly, due to
> > what is arguably a rather severe bug, issuing a native x86_64 syscall
> > (x32 bit clear) with nr in the range 512..547 does *not* return
> > -ENOSYS on a kernel with x32 enabled. Instead it does something that
> > is somewhat arbitrary. With my patch applied, it will return -ENOSYS,
> > but old kernels will still exist, and this will break syscall probing.
> >
> > Can we perhaps just start the consistent numbers above 547 or maybe
> > block out 512..547 in the new regime?
>
> I don't think you gain much with that kind of scheme - it won't take
> very long before an architecture misses having a syscall added, and
> then someone else adds their own. Been there with ARM - I was keeping
> the syscall table in the same order as x86 for new syscalls, but now
Same for m68k, and probably other architectures.
> that others have been adding syscalls to the table since I converted
> ARM to the tabular form, that's now gone out the window.
>
> So, I think it's completely pointless to do what you're suggesting.
> We'll just end up with a big hole in the middle of the syscall table
> and then revert back to random numbering of syscalls thereafter again.
I believe the plan is to add future syscalls for all architectures in a
single commit, to keep everything in sync.
Regardless, I'm wondering what to do with the holes marked "room for
arch specific calls".
When is a syscall really arch-specific, and can it be added there, and
when does it turn out (later) that it isn't, breaking the
synchronization again?
The pkey syscalls may be a bad example, as AFAIU they can be implemented
on some architectures, but not on some others. Still, I had skipped them
when adding new syscalls to m68k.
Perhaps we should get rid of the notion of "arch-specific syscalls", and
reserve a slot everywhere anyway?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 26/29] y2038: use time32 syscall names on 32-bit
From: Geert Uytterhoeven @ 2019-01-21 8:07 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038 Mailman List, Linux API, Linux Kernel Mailing List,
Linux-Arch, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Tony Luck, Fenghua Yu, Michal Simek, Paul Burton,
Helge Deller, Benjamin Herrenschmidt, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Rich Felker, David S. Miller
In-Reply-To: <20190118161835.2259170-27-arnd@arndb.de>
Hi Arnd,
On Fri, Jan 18, 2019 at 5:21 PM Arnd Bergmann <arnd@arndb.de> wrote:
> This is the big flip, where all 32-bit architectures set COMPAT_32BIT_TIME
> abd use the _time32 system calls from the former compat layer instead
and
> of the system calls that take __kernel_timespec and similar arguments.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox