On 18/02/2021 09:52, Sean Young wrote: > Hi, > > On Wed, Feb 17, 2021 at 05:29:46PM +0100, Hans Verkuil wrote: >> On 17/02/2021 16:11, Sean Young wrote: >>> 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. > > Hmm, I'm not quite sure what is happening here. How can I reproduce this > issue locally? You need the right hardware for this, I'm afraid: this issue happens if you have a DisplayPort-to-HDMI adapter that supports CEC and CONFIG_DRM_DP_CEC is set to y. In my case I have an Intel NUC with a USB-C to HDMI adapter from Club 3D (CAC-2504). I can easily test patches for you. Regards, Hans
