linuxkpi: implement irqs_disabled

[ashafer]

Summary

This adds a function which is used from various places in drm-kmod
to detect if the code can sleep or not. Currently, these locations
(such as intel_guc_send_busy_loop) are ifdeffed to freebsd specific
solutions (usually drm_can_sleep).

This change implements irqs_disabled. We check td_pinned as spinlocks
will call sched_pin, and td_critnest to check if we are in a critical
section.

[bz]

I like it and I love your detailed explanation!

I'll wait for @dumbbell as wireless does not seem to use it currently.
I am wondering how much code can/must be adjusted in the follow-up and do we do the right thing (cpu_relax)?

[ashafer]

There's a few places in drm-kmod but it's not much:

% rg -i irqs_disabled .       
./drivers/gpu/drm/i915/i915_request.c
622:	GEM_BUG_ON(!irqs_disabled());
734:	GEM_BUG_ON(!irqs_disabled());

./drivers/gpu/drm/i915/intel_runtime_pm.h
33: * The irqs_disabled variable becomes true exactly after we disable the IRQs and

./include/drm/drm_util.h
84:	if (in_atomic() || in_dbg_master() || irqs_disabled())

./drivers/gpu/drm/i915/gt/intel_engine_cs.c
1616:	if (in_atomic() || irqs_disabled()) /* inside atomic preempt-reset? */

./drivers/gpu/drm/i915/gt/uc/intel_guc.h
370:	bool not_atomic = !in_atomic() && !irqs_disabled();

[wulf]

sys/compat/linuxkpi/common/include/linux/hardirq.h

61

According to Linux docs, spin_lock() does not disable local IRQs. Only spin_lock_irq() and spin_lock_irqsave() do it.
Probably this implementation will result in false positive if ordinary spinlock is held.

Historically drm-kmod had bundled irqs_disabled() implementation that was lost during bundled->insystem LinuxKPI transition.
See commit 530462939f6. IMO we should revert it.

[ashafer]

Reverting 530462939f6 (or moving its irqs_disabled() definition somewhere else) seems reasonable to me. What was the motivation of that change originally? It sounds like just removing that header file? Is that a change you'd like to make or want me to post?

[ashafer]

@wulf any objections to this change or does it look good? I can post a revert of 530462939f6 to drm-kmod once this change lands

[wulf]

What was the motivation of that change originally?

The motivation for the change was to move failures from run-time to build-time as irqs_disabled() should be NO-OP for spinlock cases and D57782 for Intel critical section case. The one should check irqs_disabled() execution context after build failure and deceide if curthread->td_critnest > 0 check is required or not.

any objections to this change or does it look good?

I have no strong opinion so it is up to you to do the change. Consider to add local_irq_enable() and local_irq_disable() implementations to the same LKPI commit as they have similar properties. It would be good to add some comments explaining when and why these functions must be #ifdef-ed out.
Please commit drm-kmod implementation as it looks more correct.

[wulf]

And do not revert linux/kernel.h part of 530462939f6 irqs_disabled() should stay in in-base LKPI

[wulf]

I rechecked 530462939f6 again. It seems that irqs_disabled() never used in drm-kmod for spin_locks. So I am ok for old drm-kmod version of this review.

[wulf]

In D57782#1352915, @wulf wrote:

I rechecked 530462939f6 again. It seems that irqs_disabled() never used in drm-kmod for spin_locks. So I am ok for old drm-kmod version of this review.

Ooops. I was wrong. It is used in __i915_request_submit() to assert that it executed from spin_lock_irqsave() context.

[ashafer]

Thanks, updating with the old drm-kmod implementation. Also added local_irq_enable/disable since it was pretty straightforward.