On 17/02/2021 16:11, Sean Young wrote: > Hi, > > On Wed, Feb 17, 2021 at 04:04:11PM +0100, Hans de Goede wrote: >> On 2/17/21 3:32 PM, Sean Young wrote: >>> On Wed, Feb 17, 2021 at 01:41:46PM +0100, Hans Verkuil wrote: >>>> Hi Hans, >>>> >>>> On 17/02/2021 13:24, Hans de Goede wrote: >>>>> <resend with the linux-media list added to the Cc> >>>>> >>>>> Hi Hans, >>>>> >>>>> Fedora has a (opt-in) system to automatically collect backtraces from software >>>>> crashing on users systems. >>>>> >>>>> This includes collecting kernel backtraces (including once triggered by >>>>> WARN macros) while looking a the top 10 of the most reported backtrace during the >>>>> last 2 weeks report from ABRT: https://retrace.fedoraproject.org/faf/problems/ >>>>> >>>>> I noticed the following backtrace: >>>>> https://retrace.fedoraproject.org/faf/problems/8150/ >>>>> which has been reported 170000 times by Fedora users who have opted-in during the >>>>> last 14 days. >>>>> >>>>> The issue here is that cec_register_adapter ends up calling request_module() >>>>> from an async context, triggering this warn in kernel/kmod.c __request_module(): >>>>> >>>>> /* >>>>> * We don't allow synchronous module loading from async. Module >>>>> * init may invoke async_synchronize_full() which will end up >>>>> * waiting for this task which already is waiting for the module >>>>> * loading to complete, leading to a deadlock. >>>>> */ >>>>> WARN_ON_ONCE(wait && current_is_async()); >>>>> >>>>> The call-path leading to this goes like this: >>>>> >>>>> ? kvasprintf+0x6d/0xa0 >>>>> ? kobject_set_name_vargs+0x6f/0x90 >>>>> rc_map_get+0x30/0x60 >>>> >>>> It's not CEC, it is rc_map_get that calls request_module() for rc-cec.ko. >>>> >>>> I've added Sean Young to the CC list. >>>> >>>> Sean, is it possible to treat rc-cec as a built-in if MEDIA_CEC_RC is set? >>>> >>>> I think this issue is very specific to CEC. I would not expect to see this >>>> with any other rc keymap. >>> >>> So CEC creates an RC device with a keymap (cec keymap, of course) and then >>> the keymap needs to be loaded. We certainly don't want all keymaps as >>> builtins, that would be a waste. >>> >>> The cec keymap is scanned once to build a map from cec codes to linux >>> keycodes; making it builtin is not ideal, and makes the build system a >>> bit messy. >>> >>> I don't think we can load the keymap later, user space may start remapping >>> the keymap from udev. >>> >>> Possibly we could create the cec or rc device later but this could be a bit >>> messy. >>> >>> Could CEC specify: >>> >>> #if IS_ENABLED(CONFIG_MEDIA_CEC_RC) >>> MODULE_SOFTDEP("rc-cec") >>> #endif >> >> That would need to be: >> >> MODULE_SOFTDEP("pre: rc-cec") >> >> I see that the drm_kms_helper and i915 drivers both depend on the cec module already, >> so yes if that module will request for rc-cec to be loaded before it is loaded >> (and thus before i915 is loaded) then that should work around this. >> >> Assuming the user is using a module-loader which honors the softdep... >> >> Also this assumes that rc_map_get is smart enough to not call request_module() >> if the module is already loaded, is that the case ? > > Yes, see rc_map_get(). I tried this. It works if CONFIG_RC_CORE is set to m, but setting it to y resulted in the same problem. It looks like MODULE_SOFTDEP only works if rc_main is a module as well. It was a good idea, though :-) Regards, Hans
