All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/2] Allow nonatomic trigger operations
@ 2014-09-01 12:00 Takashi Iwai
  2014-09-01 12:00 ` [PATCH 1/2] ALSA: pcm: " Takashi Iwai
  2014-09-01 12:00 ` [PATCH 2/2] ALSA: pcm: Uninline snd_pcm_stream_lock() and _unlock() Takashi Iwai
  0 siblings, 2 replies; 6+ messages in thread
From: Takashi Iwai @ 2014-09-01 12:00 UTC (permalink / raw)
  To: alsa-devel

Hi,

this is a quick hack for the features that have been requested by
quite a few people since long time ago: the support of non-atomic
PCM trigger.  I've tested only with a limited driver set, so more
tests and comments/reviews are appreciated.

 [PATCH 1/2] ALSA: pcm: Allow nonatomic trigger operations
 [PATCH 2/2] ALSA: pcm: Uninline snd_pcm_stream_lock() and _unlock()

thanks,

Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] ALSA: pcm: Allow nonatomic trigger operations
  2014-09-01 12:00 [RFC][PATCH 0/2] Allow nonatomic trigger operations Takashi Iwai
@ 2014-09-01 12:00 ` Takashi Iwai
  2014-09-02 12:27   ` Jarkko Nikula
  2014-09-01 12:00 ` [PATCH 2/2] ALSA: pcm: Uninline snd_pcm_stream_lock() and _unlock() Takashi Iwai
  1 sibling, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2014-09-01 12:00 UTC (permalink / raw)
  To: alsa-devel

Currently, many PCM operations are performed in a critical section
protected by spinlock, typically the trigger and pointer callbacks are
assumed to be atomic.  This is basically because some trigger action
(e.g. PCM stop after drain or xrun) is done in the interrupt handler.
If a driver runs in a threaded irq, however, this doesn't have to be
atomic.  And many devices want to handle trigger in a non-atomic
context due to lengthy communications.

This patch tries all PCM calls operational in non-atomic context.
What it does is very simple: replaces the substream spinlock with the
corresponding substream mutex when pcm->nonatomic flag is set.  The
driver that wants to use the non-atomic PCM ops just needs to set the
flag and keep the reset as is.  (Of course, it must not handle any PCM
ops in irq context.)

Note that the code doesn't check whether it's atomic-safe or not, but
trust in 100% that the driver sets pcm->nonatomic correctly.

One possible problem is the case where linked PCM substreams have
inconsistent nonatomic states.  For avoiding this, snd_pcm_link()
returns an error if one tries to link an inconsistent PCM substream.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h     | 58 +++++++++++++++++++++++++++++--------
 sound/core/pcm.c        |  1 +
 sound/core/pcm_native.c | 76 ++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 116 insertions(+), 19 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 6f3e10ca0e32..bc79962f4aa6 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -365,6 +365,7 @@ struct snd_pcm_runtime {
 
 struct snd_pcm_group {		/* keep linked substreams */
 	spinlock_t lock;
+	struct mutex mutex;
 	struct list_head substreams;
 	int count;
 };
@@ -460,6 +461,7 @@ struct snd_pcm {
 	void (*private_free) (struct snd_pcm *pcm);
 	struct device *dev; /* actual hw device this belongs to */
 	bool internal; /* pcm is for internal use only */
+	bool nonatomic; /* whole PCM operations are in non-atomic context */
 #if defined(CONFIG_SND_PCM_OSS) || defined(CONFIG_SND_PCM_OSS_MODULE)
 	struct snd_pcm_oss oss;
 #endif
@@ -493,6 +495,7 @@ int snd_pcm_notify(struct snd_pcm_notify *notify, int nfree);
  */
 
 extern rwlock_t snd_pcm_link_rwlock;
+extern struct rw_semaphore snd_pcm_link_rwsem;
 
 int snd_pcm_info(struct snd_pcm_substream *substream, struct snd_pcm_info *info);
 int snd_pcm_info_user(struct snd_pcm_substream *substream,
@@ -539,38 +542,69 @@ static inline int snd_pcm_stream_linked(struct snd_pcm_substream *substream)
 
 static inline void snd_pcm_stream_lock(struct snd_pcm_substream *substream)
 {
-	read_lock(&snd_pcm_link_rwlock);
-	spin_lock(&substream->self_group.lock);
+	if (substream->pcm->nonatomic) {
+		down_read(&snd_pcm_link_rwsem);
+		mutex_lock(&substream->self_group.mutex);
+	} else {
+		read_lock(&snd_pcm_link_rwlock);
+		spin_lock(&substream->self_group.lock);
+	}
 }
 
 static inline void snd_pcm_stream_unlock(struct snd_pcm_substream *substream)
 {
-	spin_unlock(&substream->self_group.lock);
-	read_unlock(&snd_pcm_link_rwlock);
+	if (substream->pcm->nonatomic) {
+		mutex_unlock(&substream->self_group.mutex);
+		up_read(&snd_pcm_link_rwsem);
+	} else {
+		spin_unlock(&substream->self_group.lock);
+		read_unlock(&snd_pcm_link_rwlock);
+	}
 }
 
 static inline void snd_pcm_stream_lock_irq(struct snd_pcm_substream *substream)
 {
-	read_lock_irq(&snd_pcm_link_rwlock);
-	spin_lock(&substream->self_group.lock);
+	if (substream->pcm->nonatomic) {
+		down_read(&snd_pcm_link_rwsem);
+		mutex_lock(&substream->self_group.mutex);
+	} else {
+		read_lock_irq(&snd_pcm_link_rwlock);
+		spin_lock(&substream->self_group.lock);
+	}
 }
 
 static inline void snd_pcm_stream_unlock_irq(struct snd_pcm_substream *substream)
 {
-	spin_unlock(&substream->self_group.lock);
-	read_unlock_irq(&snd_pcm_link_rwlock);
+	if (substream->pcm->nonatomic) {
+		mutex_unlock(&substream->self_group.mutex);
+		up_read(&snd_pcm_link_rwsem);
+	} else {
+		spin_unlock(&substream->self_group.lock);
+		read_unlock_irq(&snd_pcm_link_rwlock);
+	}
 }
 
 #define snd_pcm_stream_lock_irqsave(substream, flags) \
 do { \
-	read_lock_irqsave(&snd_pcm_link_rwlock, (flags)); \
-	spin_lock(&substream->self_group.lock); \
+	if ((substream)->pcm->nonatomic) {			  \
+		(flags) = 0; /* XXX for avoid warning */	  \
+		down_read(&snd_pcm_link_rwsem);			  \
+		mutex_lock(&(substream)->self_group.mutex);	  \
+	} else {						  \
+		read_lock_irqsave(&snd_pcm_link_rwlock, (flags)); \
+		spin_lock(&(substream)->self_group.lock);	  \
+	}							  \
 } while (0)
 
 #define snd_pcm_stream_unlock_irqrestore(substream, flags) \
 do { \
-	spin_unlock(&substream->self_group.lock); \
-	read_unlock_irqrestore(&snd_pcm_link_rwlock, (flags)); \
+	if ((substream)->pcm->nonatomic) {			       \
+		mutex_unlock(&(substream)->self_group.mutex);	       \
+		up_read(&snd_pcm_link_rwsem);			       \
+	} else {						       \
+		spin_unlock(&(substream)->self_group.lock);	       \
+		read_unlock_irqrestore(&snd_pcm_link_rwlock, (flags)); \
+	}							       \
 } while (0)
 
 #define snd_pcm_group_for_each_entry(s, substream) \
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 43932e8dce66..afccdc553ef9 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -698,6 +698,7 @@ int snd_pcm_new_stream(struct snd_pcm *pcm, int stream, int substream_count)
 		}
 		substream->group = &substream->self_group;
 		spin_lock_init(&substream->self_group.lock);
+		mutex_init(&substream->self_group.mutex);
 		INIT_LIST_HEAD(&substream->self_group.substreams);
 		list_add_tail(&substream->link_list, &substream->self_group.substreams);
 		atomic_set(&substream->mmap_count, 0);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 8cd2f930ad0b..b2621aa6494c 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -77,7 +77,8 @@ static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream);
 DEFINE_RWLOCK(snd_pcm_link_rwlock);
 EXPORT_SYMBOL(snd_pcm_link_rwlock);
 
-static DECLARE_RWSEM(snd_pcm_link_rwsem);
+DECLARE_RWSEM(snd_pcm_link_rwsem);
+EXPORT_SYMBOL(snd_pcm_link_rwsem);
 
 static inline mm_segment_t snd_enter_user(void)
 {
@@ -727,9 +728,14 @@ static int snd_pcm_action_group(struct action_ops *ops,
 	int res = 0;
 
 	snd_pcm_group_for_each_entry(s, substream) {
-		if (do_lock && s != substream)
-			spin_lock_nested(&s->self_group.lock,
-					 SINGLE_DEPTH_NESTING);
+		if (do_lock && s != substream) {
+			if (s->pcm->nonatomic)
+				mutex_lock_nested(&s->self_group.mutex,
+						  SINGLE_DEPTH_NESTING);
+			else
+				spin_lock_nested(&s->self_group.lock,
+						 SINGLE_DEPTH_NESTING);
+		}
 		res = ops->pre_action(s, state);
 		if (res < 0)
 			goto _unlock;
@@ -755,8 +761,12 @@ static int snd_pcm_action_group(struct action_ops *ops,
 	if (do_lock) {
 		/* unlock streams */
 		snd_pcm_group_for_each_entry(s1, substream) {
-			if (s1 != substream)
-				spin_unlock(&s1->self_group.lock);
+			if (s1 != substream) {
+				if (s->pcm->nonatomic)
+					mutex_unlock(&s1->self_group.mutex);
+				else
+					spin_unlock(&s1->self_group.lock);
+			}
 			if (s1 == s)	/* end */
 				break;
 		}
@@ -784,6 +794,27 @@ static int snd_pcm_action_single(struct action_ops *ops,
 	return res;
 }
 
+/* call in mutex-protected context */
+static int snd_pcm_action_mutex(struct action_ops *ops,
+				struct snd_pcm_substream *substream,
+				int state)
+{
+	int res;
+
+	if (snd_pcm_stream_linked(substream)) {
+		if (!mutex_trylock(&substream->self_group.mutex)) {
+			mutex_unlock(&substream->self_group.mutex);
+			mutex_lock(&substream->group->mutex);
+			mutex_lock(&substream->self_group.mutex);
+		}
+		res = snd_pcm_action_group(ops, substream, state, 1);
+		mutex_unlock(&substream->group->mutex);
+	} else {
+		res = snd_pcm_action_single(ops, substream, state);
+	}
+	return res;
+}
+
 /*
  *  Note: call with stream lock
  */
@@ -793,6 +824,9 @@ static int snd_pcm_action(struct action_ops *ops,
 {
 	int res;
 
+	if (substream->pcm->nonatomic)
+		return snd_pcm_action_mutex(ops, substream, state);
+
 	if (snd_pcm_stream_linked(substream)) {
 		if (!spin_trylock(&substream->group->lock)) {
 			spin_unlock(&substream->self_group.lock);
@@ -807,6 +841,29 @@ static int snd_pcm_action(struct action_ops *ops,
 	return res;
 }
 
+static int snd_pcm_action_lock_mutex(struct action_ops *ops,
+				     struct snd_pcm_substream *substream,
+				     int state)
+{
+	int res;
+
+	down_read(&snd_pcm_link_rwsem);
+	if (snd_pcm_stream_linked(substream)) {
+		mutex_lock(&substream->group->mutex);
+		mutex_lock_nested(&substream->self_group.mutex,
+				  SINGLE_DEPTH_NESTING);
+		res = snd_pcm_action_group(ops, substream, state, 1);
+		mutex_unlock(&substream->self_group.mutex);
+		mutex_unlock(&substream->group->mutex);
+	} else {
+		mutex_lock(&substream->self_group.mutex);
+		res = snd_pcm_action_single(ops, substream, state);
+		mutex_unlock(&substream->self_group.mutex);
+	}
+	up_read(&snd_pcm_link_rwsem);
+	return res;
+}
+
 /*
  *  Note: don't use any locks before
  */
@@ -816,6 +873,9 @@ static int snd_pcm_action_lock_irq(struct action_ops *ops,
 {
 	int res;
 
+	if (substream->pcm->nonatomic)
+		return snd_pcm_action_lock_mutex(ops, substream, state);
+
 	read_lock_irq(&snd_pcm_link_rwlock);
 	if (snd_pcm_stream_linked(substream)) {
 		spin_lock(&substream->group->lock);
@@ -1634,7 +1694,8 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
 	down_write(&snd_pcm_link_rwsem);
 	write_lock_irq(&snd_pcm_link_rwlock);
 	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN ||
-	    substream->runtime->status->state != substream1->runtime->status->state) {
+	    substream->runtime->status->state != substream1->runtime->status->state ||
+	    substream->pcm->nonatomic != substream1->pcm->nonatomic) {
 		res = -EBADFD;
 		goto _end;
 	}
@@ -1646,6 +1707,7 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
 		substream->group = group;
 		group = NULL;
 		spin_lock_init(&substream->group->lock);
+		mutex_init(&substream->group->mutex);
 		INIT_LIST_HEAD(&substream->group->substreams);
 		list_add_tail(&substream->link_list, &substream->group->substreams);
 		substream->group->count = 1;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] ALSA: pcm: Uninline snd_pcm_stream_lock() and _unlock()
  2014-09-01 12:00 [RFC][PATCH 0/2] Allow nonatomic trigger operations Takashi Iwai
  2014-09-01 12:00 ` [PATCH 1/2] ALSA: pcm: " Takashi Iwai
@ 2014-09-01 12:00 ` Takashi Iwai
  2014-09-01 12:10   ` Takashi Iwai
  1 sibling, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2014-09-01 12:00 UTC (permalink / raw)
  To: alsa-devel

The previous commit for the non-atomic PCM ops added more codes to
snd_pcm_stream_lock() and its variants.  Since they are inlined
functions, it resulted in a significant code size bloat.  For reducing
the size bloat, this patch changes the inline functions to the normal
function calls.  The export of rwlock and rwsem are removed as well,
since they are referred only in pcm_native.c now.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h     | 83 ++++++++++---------------------------------------
 sound/core/pcm_native.c | 45 ++++++++++++++++++++++++---
 2 files changed, 58 insertions(+), 70 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index bc79962f4aa6..3e6453dc8ae0 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -494,9 +494,6 @@ int snd_pcm_notify(struct snd_pcm_notify *notify, int nfree);
  *  Native I/O
  */
 
-extern rwlock_t snd_pcm_link_rwlock;
-extern struct rw_semaphore snd_pcm_link_rwsem;
-
 int snd_pcm_info(struct snd_pcm_substream *substream, struct snd_pcm_info *info);
 int snd_pcm_info_user(struct snd_pcm_substream *substream,
 		      struct snd_pcm_info __user *info);
@@ -540,71 +537,25 @@ static inline int snd_pcm_stream_linked(struct snd_pcm_substream *substream)
 	return substream->group != &substream->self_group;
 }
 
-static inline void snd_pcm_stream_lock(struct snd_pcm_substream *substream)
-{
-	if (substream->pcm->nonatomic) {
-		down_read(&snd_pcm_link_rwsem);
-		mutex_lock(&substream->self_group.mutex);
-	} else {
-		read_lock(&snd_pcm_link_rwlock);
-		spin_lock(&substream->self_group.lock);
-	}
-}
-
-static inline void snd_pcm_stream_unlock(struct snd_pcm_substream *substream)
-{
-	if (substream->pcm->nonatomic) {
-		mutex_unlock(&substream->self_group.mutex);
-		up_read(&snd_pcm_link_rwsem);
-	} else {
-		spin_unlock(&substream->self_group.lock);
-		read_unlock(&snd_pcm_link_rwlock);
-	}
-}
-
-static inline void snd_pcm_stream_lock_irq(struct snd_pcm_substream *substream)
-{
-	if (substream->pcm->nonatomic) {
-		down_read(&snd_pcm_link_rwsem);
-		mutex_lock(&substream->self_group.mutex);
-	} else {
-		read_lock_irq(&snd_pcm_link_rwlock);
-		spin_lock(&substream->self_group.lock);
-	}
-}
-
-static inline void snd_pcm_stream_unlock_irq(struct snd_pcm_substream *substream)
-{
-	if (substream->pcm->nonatomic) {
-		mutex_unlock(&substream->self_group.mutex);
-		up_read(&snd_pcm_link_rwsem);
-	} else {
-		spin_unlock(&substream->self_group.lock);
-		read_unlock_irq(&snd_pcm_link_rwlock);
-	}
-}
-
-#define snd_pcm_stream_lock_irqsave(substream, flags) \
-do { \
-	if ((substream)->pcm->nonatomic) {			  \
-		(flags) = 0; /* XXX for avoid warning */	  \
-		down_read(&snd_pcm_link_rwsem);			  \
-		mutex_lock(&(substream)->self_group.mutex);	  \
-	} else {						  \
-		read_lock_irqsave(&snd_pcm_link_rwlock, (flags)); \
-		spin_lock(&(substream)->self_group.lock);	  \
-	}							  \
+void snd_pcm_stream_lock(struct snd_pcm_substream *substream);
+void snd_pcm_stream_unlock(struct snd_pcm_substream *substream);
+void snd_pcm_stream_lock_irq(struct snd_pcm_substream *substream);
+void snd_pcm_stream_unlock_irq(struct snd_pcm_substream *substream);
+
+#define snd_pcm_stream_lock_irqsave(substream, flags)		\
+do {								\
+	if ((substream)->pcm->nonatomic)			\
+		(flags) = 0; /* XXX for avoid warning */	\
+	else							\
+		local_irq_save(flags);				\
+	snd_pcm_stream_lock(substream);				\
 } while (0)
 
-#define snd_pcm_stream_unlock_irqrestore(substream, flags) \
-do { \
-	if ((substream)->pcm->nonatomic) {			       \
-		mutex_unlock(&(substream)->self_group.mutex);	       \
-		up_read(&snd_pcm_link_rwsem);			       \
-	} else {						       \
-		spin_unlock(&(substream)->self_group.lock);	       \
-		read_unlock_irqrestore(&snd_pcm_link_rwlock, (flags)); \
-	}							       \
+#define snd_pcm_stream_unlock_irqrestore(substream, flags)	\
+do {								\
+	snd_pcm_stream_unlock(substream);			\
+	if (!(substream)->pcm->nonatomic)			\
+		local_irq_restore(flags);			\
 } while (0)
 
 #define snd_pcm_group_for_each_entry(s, substream) \
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index b2621aa6494c..487d8ec9d43b 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -74,11 +74,48 @@ static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream);
  *
  */
 
-DEFINE_RWLOCK(snd_pcm_link_rwlock);
-EXPORT_SYMBOL(snd_pcm_link_rwlock);
+static DEFINE_RWLOCK(snd_pcm_link_rwlock);
+static DECLARE_RWSEM(snd_pcm_link_rwsem);
 
-DECLARE_RWSEM(snd_pcm_link_rwsem);
-EXPORT_SYMBOL(snd_pcm_link_rwsem);
+void snd_pcm_stream_lock(struct snd_pcm_substream *substream)
+{
+	if (substream->pcm->nonatomic) {
+		down_read(&snd_pcm_link_rwsem);
+		mutex_lock(&substream->self_group.mutex);
+	} else {
+		read_lock(&snd_pcm_link_rwlock);
+		spin_lock(&substream->self_group.lock);
+	}
+}
+EXPORT_SYMBOL_GPL(snd_pcm_stream_lock);
+
+void snd_pcm_stream_unlock(struct snd_pcm_substream *substream)
+{
+	if (substream->pcm->nonatomic) {
+		mutex_unlock(&substream->self_group.mutex);
+		up_read(&snd_pcm_link_rwsem);
+	} else {
+		spin_unlock(&substream->self_group.lock);
+		read_unlock(&snd_pcm_link_rwlock);
+	}
+}
+EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock);
+
+void snd_pcm_stream_lock_irq(struct snd_pcm_substream *substream)
+{
+	if (!substream->pcm->nonatomic)
+		local_irq_disable();
+	snd_pcm_stream_lock(substream);
+}
+EXPORT_SYMBOL_GPL(snd_pcm_stream_lock_irq);
+
+void snd_pcm_stream_unlock_irq(struct snd_pcm_substream *substream)
+{
+	snd_pcm_stream_unlock(substream);
+	if (!substream->pcm->nonatomic)
+		local_irq_enable();
+}
+EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock_irq);
 
 static inline mm_segment_t snd_enter_user(void)
 {
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ALSA: pcm: Uninline snd_pcm_stream_lock() and _unlock()
  2014-09-01 12:00 ` [PATCH 2/2] ALSA: pcm: Uninline snd_pcm_stream_lock() and _unlock() Takashi Iwai
@ 2014-09-01 12:10   ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2014-09-01 12:10 UTC (permalink / raw)
  To: alsa-devel

Oops, I sent an old version for this second patch.
Below is the latest one.  Sorry for inconvenience.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH 2/2] ALSA: pcm: Uninline snd_pcm_stream_lock() and _unlock()

The previous commit for the non-atomic PCM ops added more codes to
snd_pcm_stream_lock() and its variants.  Since they are inlined
functions, it resulted in a significant code size bloat.  For reducing
the size bloat, this patch changes the inline functions to the normal
function calls.  The export of rwlock and rwsem are removed as well,
since they are referred only in pcm_native.c now.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h     | 81 ++++++++-----------------------------------------
 sound/core/pcm_native.c | 64 +++++++++++++++++++++++++++++++++++---
 2 files changed, 72 insertions(+), 73 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index bc79962f4aa6..67e0bdb9f0fa 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -494,9 +494,6 @@ int snd_pcm_notify(struct snd_pcm_notify *notify, int nfree);
  *  Native I/O
  */
 
-extern rwlock_t snd_pcm_link_rwlock;
-extern struct rw_semaphore snd_pcm_link_rwsem;
-
 int snd_pcm_info(struct snd_pcm_substream *substream, struct snd_pcm_info *info);
 int snd_pcm_info_user(struct snd_pcm_substream *substream,
 		      struct snd_pcm_info __user *info);
@@ -540,72 +537,18 @@ static inline int snd_pcm_stream_linked(struct snd_pcm_substream *substream)
 	return substream->group != &substream->self_group;
 }
 
-static inline void snd_pcm_stream_lock(struct snd_pcm_substream *substream)
-{
-	if (substream->pcm->nonatomic) {
-		down_read(&snd_pcm_link_rwsem);
-		mutex_lock(&substream->self_group.mutex);
-	} else {
-		read_lock(&snd_pcm_link_rwlock);
-		spin_lock(&substream->self_group.lock);
-	}
-}
-
-static inline void snd_pcm_stream_unlock(struct snd_pcm_substream *substream)
-{
-	if (substream->pcm->nonatomic) {
-		mutex_unlock(&substream->self_group.mutex);
-		up_read(&snd_pcm_link_rwsem);
-	} else {
-		spin_unlock(&substream->self_group.lock);
-		read_unlock(&snd_pcm_link_rwlock);
-	}
-}
-
-static inline void snd_pcm_stream_lock_irq(struct snd_pcm_substream *substream)
-{
-	if (substream->pcm->nonatomic) {
-		down_read(&snd_pcm_link_rwsem);
-		mutex_lock(&substream->self_group.mutex);
-	} else {
-		read_lock_irq(&snd_pcm_link_rwlock);
-		spin_lock(&substream->self_group.lock);
-	}
-}
-
-static inline void snd_pcm_stream_unlock_irq(struct snd_pcm_substream *substream)
-{
-	if (substream->pcm->nonatomic) {
-		mutex_unlock(&substream->self_group.mutex);
-		up_read(&snd_pcm_link_rwsem);
-	} else {
-		spin_unlock(&substream->self_group.lock);
-		read_unlock_irq(&snd_pcm_link_rwlock);
-	}
-}
-
-#define snd_pcm_stream_lock_irqsave(substream, flags) \
-do { \
-	if ((substream)->pcm->nonatomic) {			  \
-		(flags) = 0; /* XXX for avoid warning */	  \
-		down_read(&snd_pcm_link_rwsem);			  \
-		mutex_lock(&(substream)->self_group.mutex);	  \
-	} else {						  \
-		read_lock_irqsave(&snd_pcm_link_rwlock, (flags)); \
-		spin_lock(&(substream)->self_group.lock);	  \
-	}							  \
-} while (0)
-
-#define snd_pcm_stream_unlock_irqrestore(substream, flags) \
-do { \
-	if ((substream)->pcm->nonatomic) {			       \
-		mutex_unlock(&(substream)->self_group.mutex);	       \
-		up_read(&snd_pcm_link_rwsem);			       \
-	} else {						       \
-		spin_unlock(&(substream)->self_group.lock);	       \
-		read_unlock_irqrestore(&snd_pcm_link_rwlock, (flags)); \
-	}							       \
-} while (0)
+void snd_pcm_stream_lock(struct snd_pcm_substream *substream);
+void snd_pcm_stream_unlock(struct snd_pcm_substream *substream);
+void snd_pcm_stream_lock_irq(struct snd_pcm_substream *substream);
+void snd_pcm_stream_unlock_irq(struct snd_pcm_substream *substream);
+unsigned long _snd_pcm_stream_lock_irqsave(struct snd_pcm_substream *substream);
+#define snd_pcm_stream_lock_irqsave(substream, flags)		 \
+	do {							 \
+		typecheck(unsigned long, flags);		 \
+		flags = _snd_pcm_stream_lock_irqsave(substream); \
+	} while (0)
+void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream,
+				      unsigned long flags);
 
 #define snd_pcm_group_for_each_entry(s, substream) \
 	list_for_each_entry(s, &substream->group->substreams, link_list)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index b2621aa6494c..8547fc4e1191 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -74,11 +74,67 @@ static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream);
  *
  */
 
-DEFINE_RWLOCK(snd_pcm_link_rwlock);
-EXPORT_SYMBOL(snd_pcm_link_rwlock);
+static DEFINE_RWLOCK(snd_pcm_link_rwlock);
+static DECLARE_RWSEM(snd_pcm_link_rwsem);
 
-DECLARE_RWSEM(snd_pcm_link_rwsem);
-EXPORT_SYMBOL(snd_pcm_link_rwsem);
+void snd_pcm_stream_lock(struct snd_pcm_substream *substream)
+{
+	if (substream->pcm->nonatomic) {
+		down_read(&snd_pcm_link_rwsem);
+		mutex_lock(&substream->self_group.mutex);
+	} else {
+		read_lock(&snd_pcm_link_rwlock);
+		spin_lock(&substream->self_group.lock);
+	}
+}
+EXPORT_SYMBOL_GPL(snd_pcm_stream_lock);
+
+void snd_pcm_stream_unlock(struct snd_pcm_substream *substream)
+{
+	if (substream->pcm->nonatomic) {
+		mutex_unlock(&substream->self_group.mutex);
+		up_read(&snd_pcm_link_rwsem);
+	} else {
+		spin_unlock(&substream->self_group.lock);
+		read_unlock(&snd_pcm_link_rwlock);
+	}
+}
+EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock);
+
+void snd_pcm_stream_lock_irq(struct snd_pcm_substream *substream)
+{
+	if (!substream->pcm->nonatomic)
+		local_irq_disable();
+	snd_pcm_stream_lock(substream);
+}
+EXPORT_SYMBOL_GPL(snd_pcm_stream_lock_irq);
+
+void snd_pcm_stream_unlock_irq(struct snd_pcm_substream *substream)
+{
+	snd_pcm_stream_unlock(substream);
+	if (!substream->pcm->nonatomic)
+		local_irq_enable();
+}
+EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock_irq);
+
+unsigned long _snd_pcm_stream_lock_irqsave(struct snd_pcm_substream *substream)
+{
+	unsigned long flags = 0;
+	if (!substream->pcm->nonatomic)
+		local_irq_save(flags);
+	snd_pcm_stream_lock(substream);
+	return flags;
+}
+EXPORT_SYMBOL_GPL(_snd_pcm_stream_lock_irqsave);
+
+void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream,
+				      unsigned long flags)
+{
+	snd_pcm_stream_unlock(substream);
+	if (!substream->pcm->nonatomic)
+		local_irq_restore(flags);
+}
+EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock_irqrestore);
 
 static inline mm_segment_t snd_enter_user(void)
 {
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ALSA: pcm: Allow nonatomic trigger operations
  2014-09-01 12:00 ` [PATCH 1/2] ALSA: pcm: " Takashi Iwai
@ 2014-09-02 12:27   ` Jarkko Nikula
  2014-09-02 13:06     ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Nikula @ 2014-09-02 12:27 UTC (permalink / raw)
  To: Takashi Iwai, alsa-devel

On 09/01/2014 03:00 PM, Takashi Iwai wrote:
> Currently, many PCM operations are performed in a critical section
> protected by spinlock, typically the trigger and pointer callbacks are
> assumed to be atomic.  This is basically because some trigger action
> (e.g. PCM stop after drain or xrun) is done in the interrupt handler.
> If a driver runs in a threaded irq, however, this doesn't have to be
> atomic.  And many devices want to handle trigger in a non-atomic
> context due to lengthy communications.
>
> This patch tries all PCM calls operational in non-atomic context.
> What it does is very simple: replaces the substream spinlock with the
> corresponding substream mutex when pcm->nonatomic flag is set.  The
> driver that wants to use the non-atomic PCM ops just needs to set the
> flag and keep the reset as is.  (Of course, it must not handle any PCM
>
I guess "s/reset/rest/"?

-- 
Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ALSA: pcm: Allow nonatomic trigger operations
  2014-09-02 12:27   ` Jarkko Nikula
@ 2014-09-02 13:06     ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2014-09-02 13:06 UTC (permalink / raw)
  To: Jarkko Nikula; +Cc: alsa-devel

At Tue, 02 Sep 2014 15:27:47 +0300,
Jarkko Nikula wrote:
> 
> On 09/01/2014 03:00 PM, Takashi Iwai wrote:
> > Currently, many PCM operations are performed in a critical section
> > protected by spinlock, typically the trigger and pointer callbacks are
> > assumed to be atomic.  This is basically because some trigger action
> > (e.g. PCM stop after drain or xrun) is done in the interrupt handler.
> > If a driver runs in a threaded irq, however, this doesn't have to be
> > atomic.  And many devices want to handle trigger in a non-atomic
> > context due to lengthy communications.
> >
> > This patch tries all PCM calls operational in non-atomic context.
> > What it does is very simple: replaces the substream spinlock with the
> > corresponding substream mutex when pcm->nonatomic flag is set.  The
> > driver that wants to use the non-atomic PCM ops just needs to set the
> > flag and keep the reset as is.  (Of course, it must not handle any PCM
> >
> I guess "s/reset/rest/"?

Yep.


Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-02 13:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-01 12:00 [RFC][PATCH 0/2] Allow nonatomic trigger operations Takashi Iwai
2014-09-01 12:00 ` [PATCH 1/2] ALSA: pcm: " Takashi Iwai
2014-09-02 12:27   ` Jarkko Nikula
2014-09-02 13:06     ` Takashi Iwai
2014-09-01 12:00 ` [PATCH 2/2] ALSA: pcm: Uninline snd_pcm_stream_lock() and _unlock() Takashi Iwai
2014-09-01 12:10   ` Takashi Iwai

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.