All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: alsa-devel@alsa-project.org
Cc: tiwai@suse.de, vkoul@kernel.org, broonie@kernel.org,
	Cezary Rojewski <cezary.rojewski@intel.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Subject: [RFC PATCH 0/4] ALSA: hda: potential hdac_stream locking issues?
Date: Fri, 24 Sep 2021 14:24:13 -0500	[thread overview]
Message-ID: <20210924192417.169243-1-pierre-louis.bossart@linux.intel.com> (raw)

While reviewing the HDAudio DMA handling, I found a number of
inconsistencies in how spin_locks are used. It's not clear what the
HDaudio bus->reg_lock is supposed to protect. In most cases only the
writes to specific boolean status flags are protected, and there are
multiple cases of taking the lock after testing a status flag.

This patchset suggests a more consistent locking pattern, but it's
entirely possible that the bus->reg_lock is only intented to protect
register read/write access on the HDaudio bus, and not the status
flags, and that this entire piece of code is completely
over-engineered.

On the Intel side no one knows why this spinlock was used, the reasons
are lost to history. I set the 'RFC' status on purpose in the hope
that Takashi might recall what this lock is supposed to protect. The
diff format makes this patchset difficult to review, it's recommended
to apply the patches and look at entire functions with changes to get
a better idea of the suggested changes.

Pierre-Louis Bossart (4):
  ALSA: hda: hdac_stream: fix potential locking issue in
    snd_hdac_stream_assign()
  ALSA: hda: hdac_stream: reset assigned_key in stream_release()
  ALSA: hda: hdac_ext_stream: fix potential locking issues
  ASoC: SOF: Intel: hda-dai: fix potential locking issue

 include/sound/hdaudio_ext.h     |  2 ++
 sound/hda/ext/hdac_ext_stream.c | 46 ++++++++++++++++++++-------------
 sound/hda/hdac_stream.c         |  5 ++--
 sound/soc/sof/intel/hda-dai.c   |  7 ++---
 4 files changed, 37 insertions(+), 23 deletions(-)

-- 
2.25.1


             reply	other threads:[~2021-09-24 19:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-24 19:24 Pierre-Louis Bossart [this message]
2021-09-24 19:24 ` [RFC PATCH 1/4] ALSA: hda: hdac_stream: fix potential locking issue in snd_hdac_stream_assign() Pierre-Louis Bossart
2021-09-24 19:24 ` [RFC PATCH 2/4] ALSA: hda: hdac_stream: reset assigned_key in stream_release() Pierre-Louis Bossart
2021-09-28  6:10   ` Takashi Iwai
2021-09-24 19:24 ` [RFC PATCH 3/4] ALSA: hda: hdac_ext_stream: fix potential locking issues Pierre-Louis Bossart
2021-09-24 19:24 ` [RFC PATCH 4/4] ASoC: SOF: Intel: hda-dai: fix potential locking issue Pierre-Louis Bossart
2021-09-24 19:52   ` Mark Brown
2021-09-28  8:45 ` [RFC PATCH 0/4] ALSA: hda: potential hdac_stream locking issues? Takashi Iwai

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=20210924192417.169243-1-pierre-louis.bossart@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=cezary.rojewski@intel.com \
    --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.