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(). Thanks, Sean
