[alsa-devel] [PATCH 3/7] ALSA: pcm: avoid mmap of control data if .update_appl_ptr is implemented

[Takashi Iwai]

On Fri, 30 Sep 2016 14:43:26 +0200,
Subhransu S. Prusty wrote:
> 
> From: Ramesh Babu <ramesh.babu at intel.com>
> 
> In case of mmap, by default alsa-lib mmaps both control & status data.
> 
> If driver subscribes for .appl_ptr_update, driver needs to get
> notification whenever appl ptr changes. So with control & status mmaped,
> driver won't get appl ptr notifications.
> 
> In alsa-lib IOCTL_SYNC_PTR can be forced using  sync_ptr_ioctl flag in
> conf But this makes driver behavior dependent on a flag in the conf.
> 
> This patch conditionally checks for .appl_ptr_update and returns error
> when user land asks for mmaping control & status data, thus forcing user
> to issue IOCTL_SYNC_PTR.
> 
> One drawback with this approach is, if .appl_ptr_update is subscribed by
> driver, the user space looses flexibility to mmap the control & status
> data.

Yes, and it can be seen as an obvious regression, so I'm not sure
whether this condition is the best choice.

OTOH, I now understand why you need it in this way -- alsa-lib does
mmap ctrl/status pages at snd_pcm_open() and sync_ptr is used only as
a fallback.  So, one solution would be to fix alsa-lib side.  Or, if
we fix the kernel in this way, it would work with old alsa-lib, but it
has another penalty.  Hmm...


Takashi

> Signed-off-by: Ramesh Babu <ramesh.babu at intel.com>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty at intel.com>
> ---
>  sound/core/pcm_native.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index c56d4ed..1965d83 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -3535,10 +3535,22 @@ static int snd_pcm_mmap(struct file *file, struct vm_area_struct *area)
>  	case SNDRV_PCM_MMAP_OFFSET_STATUS:
>  		if (pcm_file->no_compat_mmap)
>  			return -ENXIO;
> +		/*
> +		 * we want to force sync pointer,
> +		 * if driver implements appl_ptr_update
> +		 */
> +		if (substream->ops->appl_ptr_update)
> +			return -ENXIO;
>  		return snd_pcm_mmap_status(substream, file, area);
>  	case SNDRV_PCM_MMAP_OFFSET_CONTROL:
>  		if (pcm_file->no_compat_mmap)
>  			return -ENXIO;
> +		/*
> +		 * we want to force sync pointer,
> +		 * if driver implements appl_ptr_update
> +		 */
> +		if (substream->ops->appl_ptr_update)
> +			return -ENXIO;
>  		return snd_pcm_mmap_control(substream, file, area);
>  	default:
>  		return snd_pcm_mmap_data(substream, file, area);
> -- 
> 1.9.1
>