| Message ID | 20220308141112.880662-1-kai.vehmanen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: hda/i915 - avoid hung task timeout in i915 wait | expand |
On Tue, Mar 08, 2022 at 04:11:12PM +0200, Kai Vehmanen wrote: >If kernel is built with hung task detection enabled and >CONFIG_DEFAULT_HUNG_TASK_TIMEOUT set to less than 60 seconds, >snd_hdac_i915_init() will trigger the hung task timeout in case i915 is >not available and taint the kernel. > >Split the 60sec wait into a loop of smaller waits to avoid this. > >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >Co-developed-by: Ramalingam C <ramalingam.c@intel.com> >Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> thanks Lucas De Marchi
On 3/8/2022 3:11 PM, Kai Vehmanen wrote: > If kernel is built with hung task detection enabled and > CONFIG_DEFAULT_HUNG_TASK_TIMEOUT set to less than 60 seconds, > snd_hdac_i915_init() will trigger the hung task timeout in case i915 is > not available and taint the kernel. > > Split the 60sec wait into a loop of smaller waits to avoid this. > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Co-developed-by: Ramalingam C <ramalingam.c@intel.com> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> > --- > sound/hda/hdac_i915.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c > index 454474ac5716..6219de739b56 100644 > --- a/sound/hda/hdac_i915.c > +++ b/sound/hda/hdac_i915.c > @@ -143,7 +143,8 @@ static bool i915_gfx_present(void) > int snd_hdac_i915_init(struct hdac_bus *bus) > { > struct drm_audio_component *acomp; > - int err; > + unsigned long wait = 0; I'm not sure if it is best name for variable that is set to 0 ("false"), maybe `done` would be better? Especially looking at condition in the following for loop. > + int err, i; > > if (!i915_gfx_present()) > return -ENODEV; > @@ -159,9 +160,10 @@ int snd_hdac_i915_init(struct hdac_bus *bus) > if (!acomp->ops) { > if (!IS_ENABLED(CONFIG_MODULES) || > !request_module("i915")) { > - /* 60s timeout */ > - wait_for_completion_timeout(&acomp->master_bind_complete, > - msecs_to_jiffies(60 * 1000)); > + /* max 60s timeout */ > + for (i = 0; !wait && i < 60; i++) > + wait = wait_for_completion_timeout(&acomp->master_bind_complete, > + msecs_to_jiffies(1000)); > } > } > if (!acomp->ops) { > > base-commit: fd7698cf0858f8c5e659b655109cd93c2f15cdd3
On Tue, 08 Mar 2022 17:29:21 +0100, Amadeusz SX2awiX4ski wrote: > > On 3/8/2022 3:11 PM, Kai Vehmanen wrote: > > If kernel is built with hung task detection enabled and > > CONFIG_DEFAULT_HUNG_TASK_TIMEOUT set to less than 60 seconds, > > snd_hdac_i915_init() will trigger the hung task timeout in case i915 is > > not available and taint the kernel. > > > > Split the 60sec wait into a loop of smaller waits to avoid this. > > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > Co-developed-by: Ramalingam C <ramalingam.c@intel.com> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> > > --- > > sound/hda/hdac_i915.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c > > index 454474ac5716..6219de739b56 100644 > > --- a/sound/hda/hdac_i915.c > > +++ b/sound/hda/hdac_i915.c > > @@ -143,7 +143,8 @@ static bool i915_gfx_present(void) > > int snd_hdac_i915_init(struct hdac_bus *bus) > > { > > struct drm_audio_component *acomp; > > - int err; > > + unsigned long wait = 0; > > I'm not sure if it is best name for variable that is set to 0 > ("false"), maybe `done` would be better? Especially looking at > condition in the following for loop. Or, more simply some thing like /* max 60s timeout */ for (i = 0; i < 60; i++) { if (wait_for_completion_timeout(&acomp->master_bind_complete, msecs_to_jiffies(1000))) break; } Takashi
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c index 454474ac5716..6219de739b56 100644 --- a/sound/hda/hdac_i915.c +++ b/sound/hda/hdac_i915.c @@ -143,7 +143,8 @@ static bool i915_gfx_present(void) int snd_hdac_i915_init(struct hdac_bus *bus) { struct drm_audio_component *acomp; - int err; + unsigned long wait = 0; + int err, i; if (!i915_gfx_present()) return -ENODEV; @@ -159,9 +160,10 @@ int snd_hdac_i915_init(struct hdac_bus *bus) if (!acomp->ops) { if (!IS_ENABLED(CONFIG_MODULES) || !request_module("i915")) { - /* 60s timeout */ - wait_for_completion_timeout(&acomp->master_bind_complete, - msecs_to_jiffies(60 * 1000)); + /* max 60s timeout */ + for (i = 0; !wait && i < 60; i++) + wait = wait_for_completion_timeout(&acomp->master_bind_complete, + msecs_to_jiffies(1000)); } } if (!acomp->ops) {