From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3754FC433E7 for ; Fri, 9 Oct 2020 14:46:59 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D7185222BA for ; Fri, 9 Oct 2020 14:46:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D7185222BA Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 49AEA6ED00; Fri, 9 Oct 2020 14:46:58 +0000 (UTC) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1CFD46ED00; Fri, 9 Oct 2020 14:46:57 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id BAD94AD73; Fri, 9 Oct 2020 14:46:55 +0000 (UTC) Date: Fri, 09 Oct 2020 16:46:55 +0200 Message-ID: From: Takashi Iwai To: Kai Vehmanen In-Reply-To: <20201006161722.500256-1-kai.vehmanen@linux.intel.com> References: <20201006161722.500256-1-kai.vehmanen@linux.intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Subject: Re: [Intel-gfx] [PATCH] ALSA: hda/i915 - fix list corruption with concurrent probes X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alsa-devel@alsa-project.org, airlied@linux.ie, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, jani.nikula@intel.com, mripard@kernel.org, tzimmermann@suse.de Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, 06 Oct 2020 18:17:22 +0200, Kai Vehmanen wrote: > > From: Takashi Iwai > > Current hdac_i915 uses a static completion instance to wait > for i915 driver to complete the component bind. > > This design is not safe if multiple HDA controllers are active and > communicating with different i915 instances, and can lead to list > corruption and failed audio driver probe. > > Fix the design by moving completion mechanism to common acomp > code and remove the related code from hdac_i915. > > Co-developed-by: Kai Vehmanen > Signed-off-by: Kai Vehmanen > Signed-off-by: Takashi Iwai Now I applied it with Fixes tag to 7b882fe3e3e8 ("ALSA: hda - handle multiple i915 device instances"). thanks, Takashi > --- > include/drm/drm_audio_component.h | 4 ++++ > sound/hda/hdac_component.c | 3 +++ > sound/hda/hdac_i915.c | 23 +++-------------------- > 3 files changed, 10 insertions(+), 20 deletions(-) > > diff --git a/include/drm/drm_audio_component.h b/include/drm/drm_audio_component.h > index a45f93487039..0d36bfd1a4cd 100644 > --- a/include/drm/drm_audio_component.h > +++ b/include/drm/drm_audio_component.h > @@ -117,6 +117,10 @@ struct drm_audio_component { > * @audio_ops: Ops implemented by hda driver, called by DRM driver > */ > const struct drm_audio_component_audio_ops *audio_ops; > + /** > + * @master_bind_complete: completion held during component master binding > + */ > + struct completion master_bind_complete; > }; > > #endif /* _DRM_AUDIO_COMPONENT_H_ */ > diff --git a/sound/hda/hdac_component.c b/sound/hda/hdac_component.c > index 89126c6fd216..bb37e7e0bd79 100644 > --- a/sound/hda/hdac_component.c > +++ b/sound/hda/hdac_component.c > @@ -210,12 +210,14 @@ static int hdac_component_master_bind(struct device *dev) > goto module_put; > } > > + complete_all(&acomp->master_bind_complete); > return 0; > > module_put: > module_put(acomp->ops->owner); > out_unbind: > component_unbind_all(dev, acomp); > + complete_all(&acomp->master_bind_complete); > > return ret; > } > @@ -296,6 +298,7 @@ int snd_hdac_acomp_init(struct hdac_bus *bus, > if (!acomp) > return -ENOMEM; > acomp->audio_ops = aops; > + init_completion(&acomp->master_bind_complete); > bus->audio_component = acomp; > devres_add(dev, acomp); > > diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c > index 5f0a1aa6ad84..454474ac5716 100644 > --- a/sound/hda/hdac_i915.c > +++ b/sound/hda/hdac_i915.c > @@ -11,8 +11,6 @@ > #include > #include > > -static struct completion bind_complete; > - > #define IS_HSW_CONTROLLER(pci) (((pci)->device == 0x0a0c) || \ > ((pci)->device == 0x0c0c) || \ > ((pci)->device == 0x0d0c) || \ > @@ -130,19 +128,6 @@ static bool i915_gfx_present(void) > return pci_dev_present(ids); > } > > -static int i915_master_bind(struct device *dev, > - struct drm_audio_component *acomp) > -{ > - complete_all(&bind_complete); > - /* clear audio_ops here as it was needed only for completion call */ > - acomp->audio_ops = NULL; > - return 0; > -} > - > -static const struct drm_audio_component_audio_ops i915_init_ops = { > - .master_bind = i915_master_bind > -}; > - > /** > * snd_hdac_i915_init - Initialize i915 audio component > * @bus: HDA core bus > @@ -163,9 +148,7 @@ int snd_hdac_i915_init(struct hdac_bus *bus) > if (!i915_gfx_present()) > return -ENODEV; > > - init_completion(&bind_complete); > - > - err = snd_hdac_acomp_init(bus, &i915_init_ops, > + err = snd_hdac_acomp_init(bus, NULL, > i915_component_master_match, > sizeof(struct i915_audio_component) - sizeof(*acomp)); > if (err < 0) > @@ -177,8 +160,8 @@ int snd_hdac_i915_init(struct hdac_bus *bus) > if (!IS_ENABLED(CONFIG_MODULES) || > !request_module("i915")) { > /* 60s timeout */ > - wait_for_completion_timeout(&bind_complete, > - msecs_to_jiffies(60 * 1000)); > + wait_for_completion_timeout(&acomp->master_bind_complete, > + msecs_to_jiffies(60 * 1000)); > } > } > if (!acomp->ops) { > -- > 2.28.0 > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx