Skip to content

Commit b0f6829

Browse files
committed
20250725-wc_linuxkm_relax_long_loop: improvements from peer review: fix, clarify, and extend comments, improve indentation, and snip out a stray redundant preprocessor definition.
1 parent 77dccc0 commit b0f6829

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

linuxkm/linuxkm_wc_port.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@
360360
#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 14, 0)
361361
/* for signal_pending() */
362362
#include <linux/sched/signal.h>
363-
/* for sched_clock_cpu() */
363+
/* for local_clock() */
364364
#include <linux/sched/clock.h>
365365
#endif
366366
#include <linux/random.h>
@@ -545,7 +545,6 @@
545545

546546
#elif defined(WOLFSSL_LINUXKM_USE_SAVE_VECTOR_REGISTERS)
547547
#error WOLFSSL_LINUXKM_USE_SAVE_VECTOR_REGISTERS is set for an unsupported architecture.
548-
#define RESTORE_VECTOR_REGISTERS() WC_RELAX_LONG_LOOP();
549548
#endif /* WOLFSSL_LINUXKM_USE_SAVE_VECTOR_REGISTERS */
550549

551550
_Pragma("GCC diagnostic pop");

linuxkm/module_hooks.c

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ int wc_linuxkm_check_for_intr_signals(void) {
219219
if (sigismember(&current->pending.signal, intr_signals[i])) {
220220
#ifdef WOLFSSL_LINUXKM_VERBOSE_DEBUG
221221
pr_err("INFO: wc_linuxkm_check_for_intr_signals returning "
222-
"-EINTR on signal %d\n", intr_signals[i]);
222+
"INTERRUPTED_E on signal %d\n", intr_signals[i]);
223223
#endif
224224
return INTERRUPTED_E;
225225
}
@@ -229,25 +229,33 @@ int wc_linuxkm_check_for_intr_signals(void) {
229229
}
230230

231231
void wc_linuxkm_relax_long_loop(void) {
232-
#if WC_LINUXKM_MAX_NS_WITHOUT_YIELD >= 0
232+
#if WC_LINUXKM_MAX_NS_WITHOUT_YIELD >= 0
233233
if (preempt_count() == 0) {
234-
#if (WC_LINUXKM_MAX_NS_WITHOUT_YIELD == 0) || !defined(CONFIG_SCHED_INFO)
234+
#if (WC_LINUXKM_MAX_NS_WITHOUT_YIELD == 0) || !defined(CONFIG_SCHED_INFO)
235235
cond_resched();
236-
#else
236+
#else
237+
/* note that local_clock() wraps a local_clock_noinstr() in a
238+
* preempt_disable_notrace(), which sounds expensive but isn't --
239+
* preempt_disable_notrace() is actually just a nonlocking integer
240+
* increment of current_thread_info()->preempt.count, protected only by
241+
* various compiler optimizer barriers.
242+
*/
237243
u64 now = local_clock();
238244
u64 current_last_arrival = current->sched_info.last_arrival;
239245
s64 delta = (s64)(now - current_last_arrival);
240246
if (delta > WC_LINUXKM_MAX_NS_WITHOUT_YIELD) {
241247
cond_resched();
242-
/* note, if nothing else is runnable, cond_resched() is a no-op and
248+
/* if nothing else is runnable, cond_resched() is a no-op and
243249
* doesn't even update .last_arrival. we could force update by
244250
* sleeping, but there's no need. we've been nice enough by just
245-
* cond_resched()ing.
251+
* cond_resched()ing, and it's actually preferable to call
252+
* cond_resched() frequently once computation has looped
253+
* continuously for longer than WC_LINUXKM_MAX_NS_WITHOUT_YIELD.
246254
*/
247255
}
248-
#endif
256+
#endif
249257
}
250-
#endif
258+
#endif
251259
}
252260

253261
#if defined(WOLFSSL_LINUXKM_USE_SAVE_VECTOR_REGISTERS) && defined(CONFIG_X86)

0 commit comments

Comments
 (0)