Re: Issue with cec_register_adapter calling request_module() from an async context when called from intel_dp_detect

[Sean Young <sean@mess.org>]

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

?

Sean


> 
> Regards,
> 
> 	Hans
> 
> >  rc_register_device+0x108/0x510
> >  cec_register_adapter+0x5c/0x280 [cec]
> >  drm_dp_cec_set_edid+0x11e/0x178 [drm_kms_helper]
> >  intel_dp_set_edid+0x8d/0xc0 [i915]
> >  intel_dp_detect+0x188/0x5c0 [i915]
> >  drm_helper_probe_single_connector_modes+0xc2/0x6d0 [drm_kms_helper]
> >  ? krealloc+0x7b/0xb0
> >  drm_client_modeset_probe+0x25b/0x1320 [drm]
> >  ? kfree+0x1ea/0x200
> >  ? sched_clock+0x5/0x10
> >  ? sched_clock_cpu+0xc/0xa0
> >  __drm_fb_helper_initial_config_and_unlock+0x37/0x470 [drm_kms_helper]
> >  ? _cond_resched+0x16/0x40
> >  intel_fbdev_initial_config+0x14/0x30 [i915]
> >  async_run_entry_fn+0x39/0x160
> > 
> > So 2 questions:
> > 
> > 1. Can we get this fixed please ?
> >    Related to this, what happens if we make this an async modprobe
> >    (when running from async context) is that a problem, or is it fine
> >    if the rc_map module gets loaded later ?
> > 
> > 2. If the answer to 1. is "tricky", "maybe" or some such then can we
> > look into a workaround here ? E.g. do we know in advance which module
> > is going to be requested (1), or does that depend on the EDID data ?
> > 
> > Regards,
> > 
> > Hans
> > 
> > 
> > 1) And can we thus do tricks with a softdep on it ?
> >