diff mbox

[RFC,16/21] ALSA: pcm/oss: refer to parameters instead of copying to reduce usage of kernel stack

Message ID 20170514085756.22382-17-o-takashi@sakamocchi.jp (mailing list archive)
State New, archived
Headers show

Commit Message

Takashi Sakamoto May 14, 2017, 8:57 a.m. UTC 
Some functions in compatibility layer for Open Sound System interface has
local variable to copy some parameters in runtime of PCM substream, while
this can be replaced with reference of pointers to parameter itself. This
brings an advantage to reduce usage of kernel stack.

This commit applies this idea.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/oss/pcm_oss.c    | 16 ++++++++--------
 sound/core/oss/pcm_plugin.c |  5 +++--
 sound/core/oss/pcm_plugin.h |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

Comments

Takashi Iwai May 15, 2017, 9:10 a.m. UTC | #1 
On Sun, 14 May 2017 10:57:51 +0200,
Takashi Sakamoto wrote:
> 
> Some functions in compatibility layer for Open Sound System interface has
> local variable to copy some parameters in runtime of PCM substream, while
> this can be replaced with reference of pointers to parameter itself. This
> brings an advantage to reduce usage of kernel stack.
> 
> This commit applies this idea.

Are you sure that this patch won't break anything?
Using the local copy is for not changing the values at evaluation
before actually trying to set.


thanks,

Takashi

> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/core/oss/pcm_oss.c    | 16 ++++++++--------
>  sound/core/oss/pcm_plugin.c |  5 +++--
>  sound/core/oss/pcm_plugin.h |  2 +-
>  3 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
> index 2a47351..e306f05 100644
> --- a/sound/core/oss/pcm_oss.c
> +++ b/sound/core/oss/pcm_oss.c
> @@ -848,7 +848,7 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
>  	int direct;
>  	snd_pcm_format_t format, sformat;
>  	int n;
> -	struct snd_mask sformat_mask;
> +	const struct snd_mask *sformat_mask;
>  	struct snd_mask mask;
>  
>  	if (trylock) {
> @@ -891,18 +891,18 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
>  
>  	format = snd_pcm_oss_format_from(runtime->oss.format);
>  
> -	sformat_mask = *hw_param_mask(sparams, SNDRV_PCM_HW_PARAM_FORMAT);
> +	sformat_mask = hw_param_mask_c(sparams, SNDRV_PCM_HW_PARAM_FORMAT);
>  	if (direct)
>  		sformat = format;
>  	else
> -		sformat = snd_pcm_plug_slave_format(format, &sformat_mask);
> +		sformat = snd_pcm_plug_slave_format(format, sformat_mask);
>  
>  	if ((__force int)sformat < 0 ||
> -	    !snd_mask_test(&sformat_mask, (__force int)sformat)) {
> +	    !snd_mask_test(sformat_mask, (__force int)sformat)) {
>  		for (sformat = (__force snd_pcm_format_t)0;
>  		     (__force int)sformat <= (__force int)SNDRV_PCM_FORMAT_LAST;
>  		     sformat = (__force snd_pcm_format_t)((__force int)sformat + 1)) {
> -			if (snd_mask_test(&sformat_mask, (__force int)sformat) &&
> +			if (snd_mask_test(sformat_mask, (__force int)sformat) &&
>  			    snd_pcm_oss_format_to(sformat) >= 0)
>  				break;
>  		}
> @@ -1780,7 +1780,7 @@ static int snd_pcm_oss_get_formats(struct snd_pcm_oss_file *pcm_oss_file)
>  	int direct;
>  	struct snd_pcm_hw_params *params;
>  	unsigned int formats = 0;
> -	struct snd_mask format_mask;
> +	const struct snd_mask *format_mask;
>  	int fmt;
>  
>  	if ((err = snd_pcm_oss_get_active_substream(pcm_oss_file, &substream)) < 0)
> @@ -1802,12 +1802,12 @@ static int snd_pcm_oss_get_formats(struct snd_pcm_oss_file *pcm_oss_file)
>  		return -ENOMEM;
>  	_snd_pcm_hw_params_any(params);
>  	err = snd_pcm_hw_refine(substream, params);
> -	format_mask = *hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); 
> +	format_mask = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT);
>  	kfree(params);
>  	if (err < 0)
>  		return err;
>  	for (fmt = 0; fmt < 32; ++fmt) {
> -		if (snd_mask_test(&format_mask, fmt)) {
> +		if (snd_mask_test(format_mask, fmt)) {
>  			int f = snd_pcm_oss_format_to(fmt);
>  			if (f >= 0)
>  				formats |= f;
> diff --git a/sound/core/oss/pcm_plugin.c b/sound/core/oss/pcm_plugin.c
> index 727ac44..cadc937 100644
> --- a/sound/core/oss/pcm_plugin.c
> +++ b/sound/core/oss/pcm_plugin.c
> @@ -266,7 +266,8 @@ snd_pcm_sframes_t snd_pcm_plug_slave_size(struct snd_pcm_substream *plug, snd_pc
>  	return frames;
>  }
>  
> -static int snd_pcm_plug_formats(struct snd_mask *mask, snd_pcm_format_t format)
> +static int snd_pcm_plug_formats(const struct snd_mask *mask,
> +				snd_pcm_format_t format)
>  {
>  	struct snd_mask formats = *mask;
>  	u64 linfmts = (SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S8 |
> @@ -309,7 +310,7 @@ static snd_pcm_format_t preferred_formats[] = {
>  };
>  
>  snd_pcm_format_t snd_pcm_plug_slave_format(snd_pcm_format_t format,
> -					   struct snd_mask *format_mask)
> +					   const struct snd_mask *format_mask)
>  {
>  	int i;
>  
> diff --git a/sound/core/oss/pcm_plugin.h b/sound/core/oss/pcm_plugin.h
> index a5035c2..38e2c14 100644
> --- a/sound/core/oss/pcm_plugin.h
> +++ b/sound/core/oss/pcm_plugin.h
> @@ -126,7 +126,7 @@ int snd_pcm_plug_format_plugins(struct snd_pcm_substream *substream,
>  				struct snd_pcm_hw_params *slave_params);
>  
>  snd_pcm_format_t snd_pcm_plug_slave_format(snd_pcm_format_t format,
> -					   struct snd_mask *format_mask);
> +					   const struct snd_mask *format_mask);
>  
>  int snd_pcm_plugin_append(struct snd_pcm_plugin *plugin);
>  
> -- 
> 2.9.3
>
Takashi Iwai May 15, 2017, 9:15 a.m. UTC | #2 
On Mon, 15 May 2017 11:10:14 +0200,
Takashi Iwai wrote:
> 
> On Sun, 14 May 2017 10:57:51 +0200,
> Takashi Sakamoto wrote:
> > 
> > Some functions in compatibility layer for Open Sound System interface has
> > local variable to copy some parameters in runtime of PCM substream, while
> > this can be replaced with reference of pointers to parameter itself. This
> > brings an advantage to reduce usage of kernel stack.
> > 
> > This commit applies this idea.
> 
> Are you sure that this patch won't break anything?
> Using the local copy is for not changing the values at evaluation
> before actually trying to set.

Never mind, I understand that it's about format_mask and it's
consitfied.


Takashi

> 
> 
> thanks,
> 
> Takashi
> 
> > 
> > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > ---
> >  sound/core/oss/pcm_oss.c    | 16 ++++++++--------
> >  sound/core/oss/pcm_plugin.c |  5 +++--
> >  sound/core/oss/pcm_plugin.h |  2 +-
> >  3 files changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
> > index 2a47351..e306f05 100644
> > --- a/sound/core/oss/pcm_oss.c
> > +++ b/sound/core/oss/pcm_oss.c
> > @@ -848,7 +848,7 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
> >  	int direct;
> >  	snd_pcm_format_t format, sformat;
> >  	int n;
> > -	struct snd_mask sformat_mask;
> > +	const struct snd_mask *sformat_mask;
> >  	struct snd_mask mask;
> >  
> >  	if (trylock) {
> > @@ -891,18 +891,18 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
> >  
> >  	format = snd_pcm_oss_format_from(runtime->oss.format);
> >  
> > -	sformat_mask = *hw_param_mask(sparams, SNDRV_PCM_HW_PARAM_FORMAT);
> > +	sformat_mask = hw_param_mask_c(sparams, SNDRV_PCM_HW_PARAM_FORMAT);
> >  	if (direct)
> >  		sformat = format;
> >  	else
> > -		sformat = snd_pcm_plug_slave_format(format, &sformat_mask);
> > +		sformat = snd_pcm_plug_slave_format(format, sformat_mask);
> >  
> >  	if ((__force int)sformat < 0 ||
> > -	    !snd_mask_test(&sformat_mask, (__force int)sformat)) {
> > +	    !snd_mask_test(sformat_mask, (__force int)sformat)) {
> >  		for (sformat = (__force snd_pcm_format_t)0;
> >  		     (__force int)sformat <= (__force int)SNDRV_PCM_FORMAT_LAST;
> >  		     sformat = (__force snd_pcm_format_t)((__force int)sformat + 1)) {
> > -			if (snd_mask_test(&sformat_mask, (__force int)sformat) &&
> > +			if (snd_mask_test(sformat_mask, (__force int)sformat) &&
> >  			    snd_pcm_oss_format_to(sformat) >= 0)
> >  				break;
> >  		}
> > @@ -1780,7 +1780,7 @@ static int snd_pcm_oss_get_formats(struct snd_pcm_oss_file *pcm_oss_file)
> >  	int direct;
> >  	struct snd_pcm_hw_params *params;
> >  	unsigned int formats = 0;
> > -	struct snd_mask format_mask;
> > +	const struct snd_mask *format_mask;
> >  	int fmt;
> >  
> >  	if ((err = snd_pcm_oss_get_active_substream(pcm_oss_file, &substream)) < 0)
> > @@ -1802,12 +1802,12 @@ static int snd_pcm_oss_get_formats(struct snd_pcm_oss_file *pcm_oss_file)
> >  		return -ENOMEM;
> >  	_snd_pcm_hw_params_any(params);
> >  	err = snd_pcm_hw_refine(substream, params);
> > -	format_mask = *hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); 
> > +	format_mask = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT);
> >  	kfree(params);
> >  	if (err < 0)
> >  		return err;
> >  	for (fmt = 0; fmt < 32; ++fmt) {
> > -		if (snd_mask_test(&format_mask, fmt)) {
> > +		if (snd_mask_test(format_mask, fmt)) {
> >  			int f = snd_pcm_oss_format_to(fmt);
> >  			if (f >= 0)
> >  				formats |= f;
> > diff --git a/sound/core/oss/pcm_plugin.c b/sound/core/oss/pcm_plugin.c
> > index 727ac44..cadc937 100644
> > --- a/sound/core/oss/pcm_plugin.c
> > +++ b/sound/core/oss/pcm_plugin.c
> > @@ -266,7 +266,8 @@ snd_pcm_sframes_t snd_pcm_plug_slave_size(struct snd_pcm_substream *plug, snd_pc
> >  	return frames;
> >  }
> >  
> > -static int snd_pcm_plug_formats(struct snd_mask *mask, snd_pcm_format_t format)
> > +static int snd_pcm_plug_formats(const struct snd_mask *mask,
> > +				snd_pcm_format_t format)
> >  {
> >  	struct snd_mask formats = *mask;
> >  	u64 linfmts = (SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S8 |
> > @@ -309,7 +310,7 @@ static snd_pcm_format_t preferred_formats[] = {
> >  };
> >  
> >  snd_pcm_format_t snd_pcm_plug_slave_format(snd_pcm_format_t format,
> > -					   struct snd_mask *format_mask)
> > +					   const struct snd_mask *format_mask)
> >  {
> >  	int i;
> >  
> > diff --git a/sound/core/oss/pcm_plugin.h b/sound/core/oss/pcm_plugin.h
> > index a5035c2..38e2c14 100644
> > --- a/sound/core/oss/pcm_plugin.h
> > +++ b/sound/core/oss/pcm_plugin.h
> > @@ -126,7 +126,7 @@ int snd_pcm_plug_format_plugins(struct snd_pcm_substream *substream,
> >  				struct snd_pcm_hw_params *slave_params);
> >  
> >  snd_pcm_format_t snd_pcm_plug_slave_format(snd_pcm_format_t format,
> > -					   struct snd_mask *format_mask);
> > +					   const struct snd_mask *format_mask);
> >  
> >  int snd_pcm_plugin_append(struct snd_pcm_plugin *plugin);
> >  
> > -- 
> > 2.9.3
> >
diff mbox

Patch

diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index 2a47351..e306f05 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -848,7 +848,7 @@  static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
 	int direct;
 	snd_pcm_format_t format, sformat;
 	int n;
-	struct snd_mask sformat_mask;
+	const struct snd_mask *sformat_mask;
 	struct snd_mask mask;
 
 	if (trylock) {
@@ -891,18 +891,18 @@  static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
 
 	format = snd_pcm_oss_format_from(runtime->oss.format);
 
-	sformat_mask = *hw_param_mask(sparams, SNDRV_PCM_HW_PARAM_FORMAT);
+	sformat_mask = hw_param_mask_c(sparams, SNDRV_PCM_HW_PARAM_FORMAT);
 	if (direct)
 		sformat = format;
 	else
-		sformat = snd_pcm_plug_slave_format(format, &sformat_mask);
+		sformat = snd_pcm_plug_slave_format(format, sformat_mask);
 
 	if ((__force int)sformat < 0 ||
-	    !snd_mask_test(&sformat_mask, (__force int)sformat)) {
+	    !snd_mask_test(sformat_mask, (__force int)sformat)) {
 		for (sformat = (__force snd_pcm_format_t)0;
 		     (__force int)sformat <= (__force int)SNDRV_PCM_FORMAT_LAST;
 		     sformat = (__force snd_pcm_format_t)((__force int)sformat + 1)) {
-			if (snd_mask_test(&sformat_mask, (__force int)sformat) &&
+			if (snd_mask_test(sformat_mask, (__force int)sformat) &&
 			    snd_pcm_oss_format_to(sformat) >= 0)
 				break;
 		}
@@ -1780,7 +1780,7 @@  static int snd_pcm_oss_get_formats(struct snd_pcm_oss_file *pcm_oss_file)
 	int direct;
 	struct snd_pcm_hw_params *params;
 	unsigned int formats = 0;
-	struct snd_mask format_mask;
+	const struct snd_mask *format_mask;
 	int fmt;
 
 	if ((err = snd_pcm_oss_get_active_substream(pcm_oss_file, &substream)) < 0)
@@ -1802,12 +1802,12 @@  static int snd_pcm_oss_get_formats(struct snd_pcm_oss_file *pcm_oss_file)
 		return -ENOMEM;
 	_snd_pcm_hw_params_any(params);
 	err = snd_pcm_hw_refine(substream, params);
-	format_mask = *hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); 
+	format_mask = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT);
 	kfree(params);
 	if (err < 0)
 		return err;
 	for (fmt = 0; fmt < 32; ++fmt) {
-		if (snd_mask_test(&format_mask, fmt)) {
+		if (snd_mask_test(format_mask, fmt)) {
 			int f = snd_pcm_oss_format_to(fmt);
 			if (f >= 0)
 				formats |= f;
diff --git a/sound/core/oss/pcm_plugin.c b/sound/core/oss/pcm_plugin.c
index 727ac44..cadc937 100644
--- a/sound/core/oss/pcm_plugin.c
+++ b/sound/core/oss/pcm_plugin.c
@@ -266,7 +266,8 @@  snd_pcm_sframes_t snd_pcm_plug_slave_size(struct snd_pcm_substream *plug, snd_pc
 	return frames;
 }
 
-static int snd_pcm_plug_formats(struct snd_mask *mask, snd_pcm_format_t format)
+static int snd_pcm_plug_formats(const struct snd_mask *mask,
+				snd_pcm_format_t format)
 {
 	struct snd_mask formats = *mask;
 	u64 linfmts = (SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S8 |
@@ -309,7 +310,7 @@  static snd_pcm_format_t preferred_formats[] = {
 };
 
 snd_pcm_format_t snd_pcm_plug_slave_format(snd_pcm_format_t format,
-					   struct snd_mask *format_mask)
+					   const struct snd_mask *format_mask)
 {
 	int i;
 
diff --git a/sound/core/oss/pcm_plugin.h b/sound/core/oss/pcm_plugin.h
index a5035c2..38e2c14 100644
--- a/sound/core/oss/pcm_plugin.h
+++ b/sound/core/oss/pcm_plugin.h
@@ -126,7 +126,7 @@  int snd_pcm_plug_format_plugins(struct snd_pcm_substream *substream,
 				struct snd_pcm_hw_params *slave_params);
 
 snd_pcm_format_t snd_pcm_plug_slave_format(snd_pcm_format_t format,
-					   struct snd_mask *format_mask);
+					   const struct snd_mask *format_mask);
 
 int snd_pcm_plugin_append(struct snd_pcm_plugin *plugin);