diff mbox series

[02/30] thread_info: selector for TIF_NEED_RESCHED[_LAZY]

Message ID 20240213055554.1802415-3-ankur.a.arora@oracle.com
State New
Headers
Series PREEMPT_AUTO: support lazy rescheduling |

Commit Message

Ankur Arora Feb. 13, 2024, 5:55 a.m. UTC 
  Define tif_resched() to serve as selector for the specific
need-resched flag: tif_resched(NR_now) mapping to TIF_NEED_RESCHED
and tif_resched(NR_lazy) to TIF_NEED_RESCHED_LAZY.

Note that, for !CONFIG_PREEMPT_AUTO, tif_resched() always evaluates
to TIF_NEED_RESCHED, preserving existing scheduling behaviour.

Cc: Peter Ziljstra <peterz@infradead.org>
Originally-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/lkml/87jzshhexi.ffs@tglx/
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 include/linux/thread_info.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Thomas Gleixner Feb. 19, 2024, 3:16 p.m. UTC | #1 
On Mon, Feb 12 2024 at 21:55, Ankur Arora wrote:
>  
> +#define TIF_NEED_RESCHED_LAZY_OFFSET (TIF_NEED_RESCHED_LAZY - TIF_NEED_RESCHED)
> +
> +typedef enum {
> +	NR_now = 0,
> +	NR_lazy = 1,

Just a nitpick. Please don't camel case the constant. Can you please
write out NEED_RESCHED_* as NR is generally associated with number?

Thanks,

        tglx
Ankur Arora Feb. 20, 2024, 10:50 p.m. UTC | #2 
Thomas Gleixner <tglx@linutronix.de> writes:

> On Mon, Feb 12 2024 at 21:55, Ankur Arora wrote:
>>
>> +#define TIF_NEED_RESCHED_LAZY_OFFSET (TIF_NEED_RESCHED_LAZY - TIF_NEED_RESCHED)
>> +
>> +typedef enum {
>> +	NR_now = 0,
>> +	NR_lazy = 1,
>
> Just a nitpick. Please don't camel case the constant. Can you please
> write out NEED_RESCHED_* as NR is generally associated with number?

That's a good point. But,

__set_tsk_need_resched(NEED_RESCHED_LAZY/_NOW), or
__tif_need_resched(NEED_RESCHED_LAZY/_NOW) seems a little repetitive.
Plus, there's the risk of confusing it with the TIF_NEED_RESCHED_LAZY
flag.

How about, something like this?

 +typedef enum {
 +	RESCHED_NOW = 0,
 +	RESCHED_LAZY = 1,
 +} resched_t;

Still, not ideal. But I think it's an improvement over those two.


Thanks

--
ankur
Thomas Gleixner Feb. 21, 2024, 5:05 p.m. UTC | #3 
On Tue, Feb 20 2024 at 14:50, Ankur Arora wrote:
> How about, something like this?
>
>  +typedef enum {
>  +	RESCHED_NOW = 0,
>  +	RESCHED_LAZY = 1,
>  +} resched_t;
>
> Still, not ideal. But I think it's an improvement over those two.

Fine with me.
Steven Rostedt Feb. 21, 2024, 6:26 p.m. UTC | #4 
On Mon, 12 Feb 2024 21:55:26 -0800
Ankur Arora <ankur.a.arora@oracle.com> wrote:

> +/*
> + * tif_resched(r) maps to TIF_NEED_RESCHED[_LAZY] with CONFIG_PREEMPT_AUTO.
> + *
> + * With !CONFIG_PREEMPT_AUTO, both tif_resched(NR_now) and tif_resched(NR_lazy)
> + * reduce to the same value (TIF_NEED_RESCHED) leaving any scheduling behaviour
> + * unchanged.
> + */
> +static inline int tif_resched(resched_t rs)
> +{
> +	return TIF_NEED_RESCHED + rs * TIF_NEED_RESCHED_LAZY_OFFSET;
> +}
> +
> +static inline int _tif_resched(resched_t rs)
> +{
> +	return 1 << tif_resched(rs);
> +}
> +

This may have been mentioned in another thread (don't remember) but please
make the above __always_inline, as that also matches tif_need_resched() as
it is today.

-- Steve
Thomas Gleixner Feb. 21, 2024, 8:03 p.m. UTC | #5 
On Wed, Feb 21 2024 at 13:26, Steven Rostedt wrote:
> On Mon, 12 Feb 2024 21:55:26 -0800
> Ankur Arora <ankur.a.arora@oracle.com> wrote:
>
>> +/*
>> + * tif_resched(r) maps to TIF_NEED_RESCHED[_LAZY] with CONFIG_PREEMPT_AUTO.
>> + *
>> + * With !CONFIG_PREEMPT_AUTO, both tif_resched(NR_now) and tif_resched(NR_lazy)
>> + * reduce to the same value (TIF_NEED_RESCHED) leaving any scheduling behaviour
>> + * unchanged.
>> + */
>> +static inline int tif_resched(resched_t rs)
>> +{
>> +	return TIF_NEED_RESCHED + rs * TIF_NEED_RESCHED_LAZY_OFFSET;
>> +}
>> +
>> +static inline int _tif_resched(resched_t rs)
>> +{
>> +	return 1 << tif_resched(rs);
>> +}
>> +
>
> This may have been mentioned in another thread (don't remember) but please
> make the above __always_inline, as that also matches tif_need_resched() as
> it is today.

It's required as this is used in noinstr sections.
diff mbox series

Patch

diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 7b1d9185aac6..99043cbbb6b0 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -67,6 +67,30 @@  enum syscall_work_bit {
 #define _TIF_NEED_RESCHED_LAZY _TIF_NEED_RESCHED
 #endif
 
+#define TIF_NEED_RESCHED_LAZY_OFFSET (TIF_NEED_RESCHED_LAZY - TIF_NEED_RESCHED)
+
+typedef enum {
+	NR_now = 0,
+	NR_lazy = 1,
+} resched_t;
+
+/*
+ * tif_resched(r) maps to TIF_NEED_RESCHED[_LAZY] with CONFIG_PREEMPT_AUTO.
+ *
+ * With !CONFIG_PREEMPT_AUTO, both tif_resched(NR_now) and tif_resched(NR_lazy)
+ * reduce to the same value (TIF_NEED_RESCHED) leaving any scheduling behaviour
+ * unchanged.
+ */
+static inline int tif_resched(resched_t rs)
+{
+	return TIF_NEED_RESCHED + rs * TIF_NEED_RESCHED_LAZY_OFFSET;
+}
+
+static inline int _tif_resched(resched_t rs)
+{
+	return 1 << tif_resched(rs);
+}
+
 #ifdef __KERNEL__
 
 #ifndef arch_set_restart_data