All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Mark Brown <broonie@kernel.org>,
	alsa-devel@alsa-project.org,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	tiwai@suse.de, linux-kernel@vger.kernel.org,
	liam.r.girdwood@linux.intel.com, vkoul@kernel.org,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Jason Gunthorpe <jgg@nvidia.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [RFC PATCH 1/2] driver core: export driver_deferred_probe_trigger()
Date: Wed, 18 Aug 2021 17:28:01 +0200	[thread overview]
Message-ID: <YR0nAcC3wJd3b4Vu@kroah.com> (raw)
In-Reply-To: <14235b8d-d375-6e2d-cae9-33adf9c48120@linux.intel.com>

On Wed, Aug 18, 2021 at 09:51:51AM -0500, Pierre-Louis Bossart wrote:
> 
> 
> >>> The issue is that the driver core is using drivers completing probe as a
> >>> proxy for resources becoming available.  That works most of the time
> >>> because most probes are fully synchronous but it breaks down if a
> >>> resource provider registers resources outside of probe, we might still
> >>> be fine if system boot is still happening and something else probes but
> >>> only through luck.
> > 
> >> The driver core is not using that as a proxy, that is up to the driver
> >> itself or not.  All probe means is "yes, this driver binds to this
> >> device, thank you!" for that specific bus/class type.  That's all, if
> >> the driver needs to go off and do real work before it can properly
> >> control the device, wonderful, have it go and do that async.
> > 
> > Right, which is what is happening here - but the deferred probe
> > machinery in the core is reading more into the probe succeeding than it
> > should.
> 
> I think Greg was referring to the use of the PROBE_PREFER_ASYNCHRONOUS
> probe type. We tried just that and got a nice WARN_ON because we are
> using request_module() to deal with HDaudio codecs. The details are in
> [1] but the kernel code is unambiguous...
> 
>         /*
> 	 * 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 reason why we use a workqueue is because we are otherwise painted in
> a corner by conflicting requirements.
> 
> a) we have to use request_module()

Wait, why?

module loading is async, use auto-loading when the hardware/device is
found and reported to userspace.  Forcing a module to load by the kernel
is not always wise as the module is not always present in the filesystem
at that point in time at boot (think modules on the filesystem, not in
the initramfs).

Try fixing this issue and maybe it will resolve itself as you should be
working async.

thanks,

greg k-h

WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	tiwai@suse.de, linux-kernel@vger.kernel.org,
	liam.r.girdwood@linux.intel.com, vkoul@kernel.org,
	Mark Brown <broonie@kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Jason Gunthorpe <jgg@nvidia.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [RFC PATCH 1/2] driver core: export driver_deferred_probe_trigger()
Date: Wed, 18 Aug 2021 17:28:01 +0200	[thread overview]
Message-ID: <YR0nAcC3wJd3b4Vu@kroah.com> (raw)
In-Reply-To: <14235b8d-d375-6e2d-cae9-33adf9c48120@linux.intel.com>

On Wed, Aug 18, 2021 at 09:51:51AM -0500, Pierre-Louis Bossart wrote:
> 
> 
> >>> The issue is that the driver core is using drivers completing probe as a
> >>> proxy for resources becoming available.  That works most of the time
> >>> because most probes are fully synchronous but it breaks down if a
> >>> resource provider registers resources outside of probe, we might still
> >>> be fine if system boot is still happening and something else probes but
> >>> only through luck.
> > 
> >> The driver core is not using that as a proxy, that is up to the driver
> >> itself or not.  All probe means is "yes, this driver binds to this
> >> device, thank you!" for that specific bus/class type.  That's all, if
> >> the driver needs to go off and do real work before it can properly
> >> control the device, wonderful, have it go and do that async.
> > 
> > Right, which is what is happening here - but the deferred probe
> > machinery in the core is reading more into the probe succeeding than it
> > should.
> 
> I think Greg was referring to the use of the PROBE_PREFER_ASYNCHRONOUS
> probe type. We tried just that and got a nice WARN_ON because we are
> using request_module() to deal with HDaudio codecs. The details are in
> [1] but the kernel code is unambiguous...
> 
>         /*
> 	 * 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 reason why we use a workqueue is because we are otherwise painted in
> a corner by conflicting requirements.
> 
> a) we have to use request_module()

Wait, why?

module loading is async, use auto-loading when the hardware/device is
found and reported to userspace.  Forcing a module to load by the kernel
is not always wise as the module is not always present in the filesystem
at that point in time at boot (think modules on the filesystem, not in
the initramfs).

Try fixing this issue and maybe it will resolve itself as you should be
working async.

thanks,

greg k-h

  parent reply	other threads:[~2021-08-18 15:28 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-17 19:00 [RFC PATCH 0/2] driver core: kick deferred probe from delayed context Pierre-Louis Bossart
2021-08-17 19:00 ` Pierre-Louis Bossart
2021-08-17 19:00 ` [RFC PATCH 1/2] driver core: export driver_deferred_probe_trigger() Pierre-Louis Bossart
2021-08-17 19:00   ` Pierre-Louis Bossart
2021-08-18  5:44   ` Greg Kroah-Hartman
2021-08-18  5:44     ` Greg Kroah-Hartman
2021-08-18 11:57     ` Mark Brown
2021-08-18 11:57       ` Mark Brown
2021-08-18 13:22       ` Greg Kroah-Hartman
2021-08-18 13:22         ` Greg Kroah-Hartman
2021-08-18 13:48         ` Mark Brown
2021-08-18 13:48           ` Mark Brown
2021-08-18 14:51           ` Pierre-Louis Bossart
2021-08-18 14:59             ` Dan Williams
2021-08-18 14:59               ` Dan Williams
2021-08-18 15:28             ` Greg Kroah-Hartman [this message]
2021-08-18 15:28               ` Greg Kroah-Hartman
2021-08-18 15:53               ` Pierre-Louis Bossart
2021-08-18 15:53                 ` Pierre-Louis Bossart
2021-08-18 16:49                 ` Greg Kroah-Hartman
2021-08-18 16:49                   ` Greg Kroah-Hartman
2021-08-18 17:52                   ` Mark Brown
2021-08-18 17:52                     ` Mark Brown
2021-08-18 18:09                   ` Pierre-Louis Bossart
2021-08-18 18:28                     ` Mark Brown
2021-08-18 18:28                       ` Mark Brown
2021-08-17 19:00 ` [RFC PATCH 2/2] ASoC: SOF: trigger re-probing of deferred devices from workqueue Pierre-Louis Bossart
2021-08-17 19:00   ` Pierre-Louis Bossart
2021-08-18 12:07   ` Mark Brown
2021-08-18 12:07     ` Mark Brown
2021-08-18 15:25     ` Pierre-Louis Bossart
2021-08-18 15:25       ` Pierre-Louis Bossart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YR0nAcC3wJd3b4Vu@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=broonie@kernel.org \
    --cc=dan.j.williams@intel.com \
    --cc=geert@linux-m68k.org \
    --cc=hch@lst.de \
    --cc=jgg@nvidia.com \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=rafael@kernel.org \
    --cc=tiwai@suse.de \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.