[PATCH v5 4/6] sched_ext: Add time helpers for BPF schedulers

Changwoo Min posted 6 patches 1 year ago
There is a newer version of this series
[PATCH v5 4/6] sched_ext: Add time helpers for BPF schedulers
Posted by Changwoo Min 1 year ago
The following functions are added for BPF schedulers:
- vtime_delta(after, before)
- vtime_after(a, b)
- vtime_before(a, b)
- vtime_after_eq(a, b)
- vtime_before_eq(a, b)
- vtime_in_range(a, b, c)
- vtime_in_range_open(a, b, c)

Signed-off-by: Changwoo Min <changwoo@igalia.com>
---
 tools/sched_ext/include/scx/common.bpf.h | 94 ++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/tools/sched_ext/include/scx/common.bpf.h b/tools/sched_ext/include/scx/common.bpf.h
index 79f0798a5350..923bbf57e4f1 100644
--- a/tools/sched_ext/include/scx/common.bpf.h
+++ b/tools/sched_ext/include/scx/common.bpf.h
@@ -408,6 +408,100 @@ static __always_inline const struct cpumask *cast_mask(struct bpf_cpumask *mask)
 void bpf_rcu_read_lock(void) __ksym;
 void bpf_rcu_read_unlock(void) __ksym;
 
+/*
+ * Time helpers, most of which are from jiffies.h.
+ */
+
+/**
+ * vtime_delta - Calculate the delta between new and old time stamp
+ * @after: first comparable as u64
+ * @before: second comparable as u64
+ *
+ * Return: the time difference, which is >= 0
+ */
+static inline s64 vtime_delta(u64 after, u64 before)
+{
+	return (s64)(after - before) > 0 ? : 0;
+}
+
+/**
+ * vtime_after - returns true if the time a is after time b.
+ * @a: first comparable as u64
+ * @b: second comparable as u64
+ *
+ * Do this with "<0" and ">=0" to only test the sign of the result. A
+ * good compiler would generate better code (and a really good compiler
+ * wouldn't care). Gcc is currently neither.
+ *
+ * Return: %true is time a is after time b, otherwise %false.
+ */
+static inline bool vtime_after(u64 a, u64 b)
+{
+	 return (s64)(b - a) < 0;
+}
+
+/**
+ * vtime_before - returns true if the time a is before time b.
+ * @a: first comparable as u64
+ * @b: second comparable as u64
+ *
+ * Return: %true is time a is before time b, otherwise %false.
+ */
+static inline bool vtime_before(u64 a, u64 b)
+{
+	return vtime_after(b, a);
+}
+
+/**
+ * vtime_after_eq - returns true if the time a is after or the same as time b.
+ * @a: first comparable as u64
+ * @b: second comparable as u64
+ *
+ * Return: %true is time a is after or the same as time b, otherwise %false.
+ */
+static inline bool vtime_after_eq(u64 a, u64 b)
+{
+	 return (s64)(a - b) >= 0;
+}
+
+/**
+ * vtime_before_eq - returns true if the time a is before or the same as time b.
+ * @a: first comparable as u64
+ * @b: second comparable as u64
+ *
+ * Return: %true is time a is before or the same as time b, otherwise %false.
+ */
+static inline bool vtime_before_eq(u64 a, u64 b)
+{
+	return vtime_after_eq(b, a);
+}
+
+/**
+ * vtime_in_range - Calculate whether a is in the range of [b, c].
+ * @a: time to test
+ * @b: beginning of the range
+ * @c: end of the range
+ *
+ * Return: %true is time a is in the range [b, c], otherwise %false.
+ */
+static inline bool vtime_in_range(u64 a, u64 b, u64 c)
+{
+	return vtime_after_eq(a, b) && vtime_before_eq(a, c);
+}
+
+/**
+ * vtime_in_range_open - Calculate whether a is in the range of [b, c).
+ * @a: time to test
+ * @b: beginning of the range
+ * @c: end of the range
+ *
+ * Return: %true is time a is in the range [b, c), otherwise %false.
+ */
+static inline bool vtime_in_range_open(u64 a, u64 b, u64 c)
+{
+	return vtime_after_eq(a, b) && vtime_before(a, c);
+}
+
 
 /*
  * Other helpers
-- 
2.47.1
Re: [PATCH v5 4/6] sched_ext: Add time helpers for BPF schedulers
Posted by Andrea Righi 1 year ago
Hi Changwoo,

On Mon, Dec 16, 2024 at 12:11:42PM +0900, Changwoo Min wrote:
> The following functions are added for BPF schedulers:
> - vtime_delta(after, before)
> - vtime_after(a, b)
> - vtime_before(a, b)
> - vtime_after_eq(a, b)
> - vtime_before_eq(a, b)
> - vtime_in_range(a, b, c)
> - vtime_in_range_open(a, b, c)

Considering that we sync these headers and sched examples from the scx repo
(see https://github.com/sched-ext/scx/blob/main/scheds/sync-to-kernel.sh),
maybe we could have a corresponding change there as well and include a link
to the PR here (as "Link: ...").

Moreover, tis particular change doesn't require to have the rest of the
patch set applied to the kernel, so a PR can be created even now. In this
way we don't risk to get out of sync.

What do you think?

Thanks,
-Andrea

> 
> Signed-off-by: Changwoo Min <changwoo@igalia.com>
> ---
>  tools/sched_ext/include/scx/common.bpf.h | 94 ++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
> 
> diff --git a/tools/sched_ext/include/scx/common.bpf.h b/tools/sched_ext/include/scx/common.bpf.h
> index 79f0798a5350..923bbf57e4f1 100644
> --- a/tools/sched_ext/include/scx/common.bpf.h
> +++ b/tools/sched_ext/include/scx/common.bpf.h
> @@ -408,6 +408,100 @@ static __always_inline const struct cpumask *cast_mask(struct bpf_cpumask *mask)
>  void bpf_rcu_read_lock(void) __ksym;
>  void bpf_rcu_read_unlock(void) __ksym;
>  
> +/*
> + * Time helpers, most of which are from jiffies.h.
> + */
> +
> +/**
> + * vtime_delta - Calculate the delta between new and old time stamp
> + * @after: first comparable as u64
> + * @before: second comparable as u64
> + *
> + * Return: the time difference, which is >= 0
> + */
> +static inline s64 vtime_delta(u64 after, u64 before)
> +{
> +	return (s64)(after - before) > 0 ? : 0;
> +}
> +
> +/**
> + * vtime_after - returns true if the time a is after time b.
> + * @a: first comparable as u64
> + * @b: second comparable as u64
> + *
> + * Do this with "<0" and ">=0" to only test the sign of the result. A
> + * good compiler would generate better code (and a really good compiler
> + * wouldn't care). Gcc is currently neither.
> + *
> + * Return: %true is time a is after time b, otherwise %false.
> + */
> +static inline bool vtime_after(u64 a, u64 b)
> +{
> +	 return (s64)(b - a) < 0;
> +}
> +
> +/**
> + * vtime_before - returns true if the time a is before time b.
> + * @a: first comparable as u64
> + * @b: second comparable as u64
> + *
> + * Return: %true is time a is before time b, otherwise %false.
> + */
> +static inline bool vtime_before(u64 a, u64 b)
> +{
> +	return vtime_after(b, a);
> +}
> +
> +/**
> + * vtime_after_eq - returns true if the time a is after or the same as time b.
> + * @a: first comparable as u64
> + * @b: second comparable as u64
> + *
> + * Return: %true is time a is after or the same as time b, otherwise %false.
> + */
> +static inline bool vtime_after_eq(u64 a, u64 b)
> +{
> +	 return (s64)(a - b) >= 0;
> +}
> +
> +/**
> + * vtime_before_eq - returns true if the time a is before or the same as time b.
> + * @a: first comparable as u64
> + * @b: second comparable as u64
> + *
> + * Return: %true is time a is before or the same as time b, otherwise %false.
> + */
> +static inline bool vtime_before_eq(u64 a, u64 b)
> +{
> +	return vtime_after_eq(b, a);
> +}
> +
> +/**
> + * vtime_in_range - Calculate whether a is in the range of [b, c].
> + * @a: time to test
> + * @b: beginning of the range
> + * @c: end of the range
> + *
> + * Return: %true is time a is in the range [b, c], otherwise %false.
> + */
> +static inline bool vtime_in_range(u64 a, u64 b, u64 c)
> +{
> +	return vtime_after_eq(a, b) && vtime_before_eq(a, c);
> +}
> +
> +/**
> + * vtime_in_range_open - Calculate whether a is in the range of [b, c).
> + * @a: time to test
> + * @b: beginning of the range
> + * @c: end of the range
> + *
> + * Return: %true is time a is in the range [b, c), otherwise %false.
> + */
> +static inline bool vtime_in_range_open(u64 a, u64 b, u64 c)
> +{
> +	return vtime_after_eq(a, b) && vtime_before(a, c);
> +}
> +
>  
>  /*
>   * Other helpers
> -- 
> 2.47.1
>
Re: [PATCH v5 4/6] sched_ext: Add time helpers for BPF schedulers
Posted by Changwoo Min 1 year ago
Hello,

On 24. 12. 16. 16:36, Andrea Righi wrote:
> Hi Changwoo,
> 
> On Mon, Dec 16, 2024 at 12:11:42PM +0900, Changwoo Min wrote:
>> The following functions are added for BPF schedulers:
>> - vtime_delta(after, before)
>> - vtime_after(a, b)
>> - vtime_before(a, b)
>> - vtime_after_eq(a, b)
>> - vtime_before_eq(a, b)
>> - vtime_in_range(a, b, c)
>> - vtime_in_range_open(a, b, c)
> 
> Considering that we sync these headers and sched examples from the scx repo
> (see https://github.com/sched-ext/scx/blob/main/scheds/sync-to-kernel.sh),
> maybe we could have a corresponding change there as well and include a link
> to the PR here (as "Link: ...").
> 
> Moreover, tis particular change doesn't require to have the rest of the
> patch set applied to the kernel, so a PR can be created even now. In this
> way we don't risk to get out of sync.
> 
> What do you think?

Thanks for the suggestion! As soon as there is a consensus,
I will submit a PR on the scx repo for the common.bpf.h and the
affected scx schedulers.

Regards,
Changwoo Min