RE: [PATCH 8/8] staging: tidspbridge - make sync_wait_on_event interruptible

["Guzman Lugo, Fernando" <fernando.lugo@ti.com>]

 

> -----Original Message-----
> From: Felipe Contreras [mailto:felipe.contreras@nokia.com] 
> Sent: Tuesday, October 26, 2010 2:27 PM
> To: Guzman Lugo, Fernando; felipe.contreras@gmail.com
> Cc: gregkh@suse.de; hiroshi.doyu@nokia.com; 
> linux-kernel@vger.kernel.org; andy.shevchenko@gmail.com; 
> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: RE: [PATCH 8/8] staging: tidspbridge - make 
> sync_wait_on_event interruptible
> 
> fernando.lugo@ti.com wrote:
> > > fernando.lugo@ti.com wrote:
> > > > > On Tue, Oct 26, 2010 at 3:51 AM, Fernando Guzman Lugo 
> > > > > <x0095840@ti.com> wrote:
> > > > > > So that avoid non-killable process.
> > > > > 
> > > > > It would be useful to interrupt these tasks from user-space. 
> > > > > A separate ioctl to do that would be needed.
> > > > 
> > > > I don't see use case where that could be needed. It is only
> > > To avoid a
> > > > nonkillable task in the case the user pass an infinite Timeout.
> > > > 
> > > > If you have some test case where that ioctl would be 
> needed Please 
> > > > share it in order to find the best solution.
> > > 
> > > Well, imagine the application is using a library to 
> access the DSP, 
> > > and the library has a dedicated thread listening for DSP 
> events in a 
> > > loop.
> > > This happens to be how libomxil-ti and gst-dsp work.
> > > 
> > > Now, the thread received the last message, but has set a 
> timeout of 
> > > 10s, or even worst, no timeout at all.
> > > 
> > > After realizing that was the last message, the main 
> thread decides 
> > > to shut down, but it has to wait for the DSP thread to join. 
> > > Unfortunately the DSP thread is stuck waiting for events, and 
> > > there's nothing that can be done.
> > > 
> > > However, if we have a separate ioctl to interrupt that task, then 
> > > the main thread can issue that ioctl, and unlock the DSP thread 
> > > without having to wait 10s, or forever.
> > > 
> > > Does that make sense?
> > 
> > Maybe sending a signal to yourselft and having a dummy 
> signal Handle 
> > should work, it that would not like good.
> 
> Signals on libraries is a no-no.
> 
> > I am thinking On having a ioctl to create and set an event the you 
> > could Something like this:
> > 
> > struct dsp_notification events[3];
> > 
> > proc_register_notify(proc, event_type, &events[0]); ...
> > proc_register_notify(proc, event_type, &events[1]); ...
> > Sync_open_event(&events[2]);
> > 
> > 
> > second thread:
> > 
> > 	mgr_wait_for_bridge_events(proc, events, 3, index);
> > 
> > 	if (index == 2) 
> > 		/* main thread force exit */
> > 
> > 
> > Main thread:
> > 
> > 	/* if some execption happened then finish the second thread */
> > 	sync_set_event(events[2]);
> > 
> > 	pthread_join(...);
> > 
> > 
> > However it is in progess a task for change replacing 
> dspbridge sync.c 
> > Module with event_fd to signal events to userspace. Where 
> now simple 
> > File descriptor will be used as event elements. So the 
> > mgr_wait_for_bridge_events Will be implemented using 
> "select" system call inside to wait for multiple events.
> > So you will be able to do something like this:
> > 
> > int events[3];
> > 
> > proc_register_notify(proc, event_type, &events[0]); ...
> > proc_register_notify(proc, event_type, &events[1]); ...
> > events[2] = eventfd(0, 0);
> > 
> > 
> > second thread:
> > 
> > 	mgr_wait_for_bridge_events(proc, events, 3, index);
> > 
> > 	if (index == 2) 
> > 		/* main thread force exit */
> > 
> > Main thread:
> > 
> > 	/* if some execption happened then finish the second thread */
> > 	write(events[2], "s", 1);
> > 
> > 	pthread_join(...);
> > 
> > You won't need any aditional ioctl in order to do what you 
> want to do.
> > 
> > So, I think it is not worth to make much changes to some 
> module that 
> > will Dissapear (my patch is just a fix it is not implementing 
> > something new), It is just a matter of time to that task is 
> finished 
> > and tested properly And then send to LO.
> 
> All right, that makes sense. I just wanted to make suere you 
> are aware of that need.

Sure, that need will cover soon.

Thanks and regards,
Fernando.

> 
> --
> Felipe Contreras
>