Messages in this thread | ![/](/images/icornerl.gif) | | From | Ankur Arora <> | Subject | Re: [PATCH 03/30] thread_info: tif_need_resched() now takes resched_t as param | Date | Tue, 20 Feb 2024 14:21:16 -0800 |
| |
Thomas Gleixner <tglx@linutronix.de> writes:
> On Wed, Feb 14 2024 at 14:08, Mark Rutland wrote: >> On Mon, Feb 12, 2024 at 09:55:27PM -0800, Ankur Arora wrote: >>> >>> -static __always_inline bool tif_need_resched(void) >>> +static __always_inline bool __tif_need_resched(int nr_flag) >>> { >>> - return test_bit(TIF_NEED_RESCHED, >>> - (unsigned long *)(¤t_thread_info()->flags)); >>> + return test_bit(nr_flag, >>> + (unsigned long *)(¤t_thread_info()->flags)); >>> } >>> >>> #endif /* _ASM_GENERIC_BITOPS_INSTRUMENTED_NON_ATOMIC_H */ >>> >>> +static __always_inline bool tif_need_resched(resched_t rs) >>> +{ >>> + /* >>> + * With !PREEMPT_AUTO tif_need_resched(NR_lazy) is defined >>> + * as TIF_NEED_RESCHED (the TIF_NEED_RESCHED_LAZY flag is not >>> + * defined). Return false in that case. >>> + */ >>> + if (IS_ENABLED(CONFIG_PREEMPT_AUTO) || rs == NR_now) >>> + return __tif_need_resched(tif_resched(rs)); >>> + else >>> + return false; >>> +} >> >> As above, I think this would be a bit simpler/clearer if we did: >> >> static __always_inline bool tif_need_resched_now(void) >> { >> return __tif_need_resched(TIF_NEED_RESCHED); >> } >> >> static __always_inline bool tif_need_resched_lazy(void) >> { >> return IS_ENABLED(CONFIG_PREEMPT_AUTO) && >> __tif_need_resched(TIF_NEED_RESCHED_LAZY); >> } > > Yes please.
As I wrote to Mark in the sibling subthread, I think exposing the lazy variants outside of the scheduler isn't really needed.
Non-scheduler/non-entry code only cares about checking if it needs to do reschedule now. So, I think we can get away with just having:
static __always_inline bool __tif_need_resched(resched_t rs) { /* * With !PREEMPT_AUTO tif_need_resched(NR_lazy) is defined * as TIF_NEED_RESCHED (the TIF_NEED_RESCHED_LAZY flag is not * defined). Return false in that case. */ if (IS_ENABLED(CONFIG_PREEMPT_AUTO) || rs == NR_now) return tif_need_resched_bitop(tif_resched(rs)); else return false; }
static __always_inline bool tif_need_resched(void) { return __tif_need_resched(NR_now); }
(and similar for all the other interfaces.)
This way lazy and eager just becomes an implementation detail which which seems to also match nicely to the point of PREEMPT_AUTO.
-- ankur
| ![\](/images/icornerr.gif) |