[alsa-devel] [PATCH 1/3] ALSA: pcm: implement the anonymous dup (inode file descriptor)

[Jaroslav Kysela]

Dne 29.1.2019 v 19:44 Takashi Iwai napsal(a):
> On Tue, 29 Jan 2019 18:59:07 +0100,
> Jaroslav Kysela wrote:
>>
>> This patch implements new SNDRV_PCM_IOCTL_ANONYMOUS_DUP ioctl which
>> returns the new duplicated anonymous inode file descriptor
>> (anon_inode:snd-pcm) which can be passed to the restricted clients.
>>
>> This patch is meant to be the alternative for the dma-buf interface. Both
>> implementation have some pros and cons:
>>
>> anon_inode:dmabuf
>>
>> - a bit standard export API for the DMA buffers
>> - fencing for the concurrent access [1]
>> - driver/kernel interface for the DMA buffer [1]
>> - multiple attach/detach scheme [1]
>>
>> [1] the real usage for the sound PCM is unknown at the moment for this feature
>>
>> anon_inode:snd-pcm
>>
>> - simple (no problem with ref-counting, non-standard mmap implementation etc.)
>> - allow to use more sound interfaces for the file descriptor like status ioctls
>> - more fine grained security policies (another anon_inode name unshared with
>>   other drivers)
> 
> Your sign-off seems missing.

Fixed now. Thanks.

>> @@ -999,15 +1002,19 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
>>  	}
>>  
>>  	if (file->f_flags & O_APPEND) {
>> -		if (prefer_subdevice < 0) {
>> -			if (pstr->substream_count > 1)
>> -				return -EINVAL; /* must be unique */
>> -			substream = pstr->substream;
>> +		if (clone) {
>> +			substream = clone;
>>  		} else {
>> -			for (substream = pstr->substream; substream;
>> -			     substream = substream->next)
>> -				if (substream->number == prefer_subdevice)
>> -					break;
>> +			if (prefer_subdevice < 0) {
>> +				if (pstr->substream_count > 1)
>> +					return -EINVAL; /* must be unique */
>> +				substream = pstr->substream;
>> +			} else {
>> +				for (substream = pstr->substream; substream;
>> +				     substream = substream->next)
>> +					if (substream->number == prefer_subdevice)
>> +						break;
>> +			}
>>  		}
>>  		if (! substream)
>>  			return -ENODEV;
> 
> So the clone case should return via this block, then...
> 
>> @@ -1018,11 +1025,18 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
>>  		return 0;
>>  	}
>>  
>> -	for (substream = pstr->substream; substream; substream = substream->next) {
>> -		if (!SUBSTREAM_BUSY(substream) &&
>> -		    (prefer_subdevice == -1 ||
>> -		     substream->number == prefer_subdevice))
>> -			break;
>> +	if (clone) {
>> +		substream = clone;
>> +		if (SUBSTREAM_BUSY(substream))
>> +			return -EAGAIN;
>> +	} else {
>> +		for (substream = pstr->substream; substream;
>> +		     substream = substream->next) {
>> +			if (!SUBSTREAM_BUSY(substream) &&
>> +			    (prefer_subdevice == -1 ||
>> +			     substream->number == prefer_subdevice))
>> +				break;
>> +		}
> 
> ... do we need to support cloning without O_APPEND?

I think that it would be better to pass the subdevice number to
snd_pcm_attach_substream(). It will reduce the required modifications.
I will change this.

				Thanks,
					Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.