linux-kernel - Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <s5hbnp5z9uy.wl-tiwai@suse.de>
Date:	Tue, 21 Oct 2014 18:05:41 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Hans Verkuil <hverkuil@...all.nl>
Cc:	Shuah Khan <shuahkh@....samsung.com>,
	Lars-Peter Clausen <lars@...afoo.de>, m.chehab@...sung.com,
	akpm@...ux-foundation.org, gregkh@...uxfoundation.org,
	crope@....fi, olebowle@....com, dheitmueller@...nellabs.com,
	ramakrmu@...co.com, sakari.ailus@...ux.intel.com,
	laurent.pinchart@...asonboard.com, perex@...ex.cz,
	prabhakar.csengg@...il.com, tim.gardner@...onical.com,
	linux@...elenboom.it, linux-kernel@...r.kernel.org,
	alsa-devel@...a-project.org, linux-media@...r.kernel.org
Subject: Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

At Tue, 21 Oct 2014 17:42:51 +0200,
Hans Verkuil wrote:
> 
> 
> 
> On 10/16/2014 04:48 PM, Takashi Iwai wrote:
> > At Thu, 16 Oct 2014 08:39:14 -0600,
> > Shuah Khan wrote:
> >>
> >> On 10/16/2014 08:16 AM, Takashi Iwai wrote:
> >>> At Thu, 16 Oct 2014 08:10:52 -0600,
> >>> Shuah Khan wrote:
> >>>>
> >>>> On 10/16/2014 08:01 AM, Takashi Iwai wrote:
> >>>>> At Thu, 16 Oct 2014 07:10:37 -0600,
> >>>>> Shuah Khan wrote:
> >>>>>>
> >>>>>> On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
> >>>>>>> On 10/14/2014 04:58 PM, Shuah Khan wrote:
> >>>>>>> [...]
> >>>>>>>>        switch (cmd) {
> >>>>>>>>        case SNDRV_PCM_TRIGGER_START:
> >>>>>>>> +        err = media_get_audio_tkn(&subs->dev->dev);
> >>>>>>>> +        if (err == -EBUSY) {
> >>>>>>>> +            dev_info(&subs->dev->dev, "%s device is busy\n",
> >>>>>>>> +                    __func__);
> >>>>>>>
> >>>>>>> In my opinion this should not dev_info() as this is out of band error
> >>>>>>> signaling and also as the potential to spam the log. The userspace
> >>>>>>> application is already properly notified by the return code.
> >>>>>>>
> >>>>>>
> >>>>>> Yes it has the potential to flood the dmesg especially with alsa,
> >>>>>> I will remove the dev_info().
> >>>>>
> >>>>> Yes.  And, I think doing this in the trigger isn't the best.
> >>>>> Why not in open & close?
> >>>>
> >>>> My first cut of this change was in open and close. I saw pulseaudio
> >>>> application go into this loop trying to open the device. To avoid
> >>>> such problems, I went with trigger stat and stop. That made all the
> >>>> pulseaudio continues attempts to open problems go away.
> >>>
> >>> But now starting the stream gives the error, and PA would loop it
> >>> again, no?
> >>>
> >>>
> >>>>> Also, is this token restriction needed only for PCM?  No mixer or
> >>>>> other controls?
> >>>>
> >>>> snd_pcm_ops are the only ones media drivers implement and use. So
> >>>> I don't think mixer is needed. If it is needed, it is not to hard
> >>>> to add for that case.
> >>>
> >>> Well, then I wonder what resource does actually conflict with
> >>> usb-audio and media drivers at all...?
> >>>
> >>
> >> audio for dvb/v4l apps gets disrupted when audio app starts. For
> >> example, dvb or v4l app tuned to a channel, and when an audio app
> >> starts. audio path needs protected to avoid conflicts between
> >> digital and analog applications as well.
> >
> > OK, then concentrating on only PCM is fine.
> >
> > But, I'm still not convinced about doing the token management in the
> > trigger.  The reason -EBUSY doesn't work is that it's the very same
> > error code when a PCM device is blocked by other processes.  And
> > -EAGAIN is interpreted by PCM core to -EBUSY for historical reasons.
> >
> > How applications (e.g. PA) should behave if the token get fails?
> > Shouldn't it retry or totally give up?
> 
> Quite often media apps open the alsa device at the start and then switch
> between TV, radio or DVB mode. If the alsa device would claim the tuner
> just by being opened (as opposed to actually using the tuner, which happens
> when you start streaming),

What about parameter changes?  The sound devices have to be configured
before using.  Don't they influence on others at all, i.e. you can
change the PCM sample rate etc during TV, radio or DVB is running?

> then that would make it impossible for the
> application to switch tuner mode. In general you want to avoid that open()
> will start configuring hardware since that can quite often be slow. Tuner
> configuration in particular can be slow since several common tuners need
> to load firmware over i2c. You only want to do that when it is really needed,
> and not when some application (udev!) opens the device just to examine what
> sort of device it is.

But most apps close the device soon after that, no?
Which programs keep the PCM device (not the control) opened without
actually using?

> So claiming the tuner in the trigger seems to be the right place. If
> returning EBUSY is a poor error code for alsa, then we can use something else
> for that. EACCES perhaps?

Sorry, I'm not convinced by that.  If the device has to be controlled
exclusively, the right position is the open/close.  Otherwise, the
program cannot know when it becomes inaccessible out of sudden during
its operation.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/