[Intel-gfx] Issue with cec_register_adapter calling request_module() from an async context when called from intel_dp_detect

[Hans Verkuil]

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