From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vinod Koul Subject: Re: [PATCH v3] ALSA: compress: fix drain calls blocking other compress functions Date: Wed, 30 Oct 2013 21:03:54 +0530 Message-ID: <20131030153354.GE18788@intel.com> References: <1382600141-9866-1-git-send-email-vinod.koul@intel.com> <1382612851-18058-1-git-send-email-vinod.koul@intel.com> <20131030145811.GC18788@intel.com> <20131030151840.GD18788@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by alsa0.perex.cz (Postfix) with ESMTP id 7A561265463 for ; Wed, 30 Oct 2013 17:28:28 +0100 (CET) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Iwai Cc: alsa-devel@alsa-project.org, broonie@kernel.org, lgirdwood@gmail.com List-Id: alsa-devel@alsa-project.org On Wed, Oct 30, 2013 at 05:24:12PM +0100, Takashi Iwai wrote: > At Wed, 30 Oct 2013 20:48:40 +0530, > Vinod Koul wrote: > > > > On Wed, Oct 30, 2013 at 05:01:38PM +0100, Takashi Iwai wrote: > > > > > > > > +static int snd_compress_wait_for_drain(struct snd_compr_stream *stream) > > > > > > +{ > > > > > > + /* > > > > > > + * We are called with lock held. So drop the lock while we wait for > > > > > > + * drain complete notfication from the driver > > > > > > + * > > > > > > + * It is expected that driver will notify the drain completion and then > > > > > > + * stream will be moved to SETUP state, even if draining resulted in an > > > > > > + * error. We can trigger next track after this. > > > > > > + */ > > > > > > + stream->runtime->state = SNDRV_PCM_STATE_DRAINING; > > > > > > + mutex_unlock(&stream->device->lock); > > > > > > + > > > > > > + wait_event(stream->runtime->wait, stream->runtime->drain_wake); > > > > > > > > > > Don't you really want the interruption? > > > > > Or use wait_event_interruptible() and return error appropriately. > > > > any interruption from usermode should trigger the compress_stop/compress_free > > > > > > ... but how? > > > > > > > which will unblock this. I dont see the need to have interruptible here > > > > > > Suppose you're running a single thread program. Then who triggers the > > > stop? > > Your signal catcher which should be registered for signals intrested and then > > calls stop or free. Assuming this should be considered single threaded. IIRC, > > this is how you do in aplay, right? > > No, issuing an ioctl in a signal isn't guaranteed to work at all. Okay thanks for pointing out, do you know why we have that limitation. Btw closing the fd should have same effect as free would trigger the stop internally > You shouldn't code the kernel driver relying on that userspace > behavior. Ok, will update it -- ~Vinod