Re: [PATCH 8/8] staging: tidspbridge - make sync_wait_on_event interruptible
On Mon, Oct 25, 2010 at 5:17 PM, Guzman Lugo, Fernando wrote: > So that avoid nonkillable process. > > Signed-off-by: Fernando Guzman Lugo > --- > .../staging/tidspbridge/include/dspbridge/sync.h | 13 +++-- > 1 files changed, 11 insertions(+), 2 deletions(-) Pushed to dspbridge. Regards, Omar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/8] staging: tidspbridge - make sync_wait_on_event interruptible
> -Original Message- > From: Felipe Contreras [mailto:felipe.contre...@nokia.com] > Sent: Tuesday, October 26, 2010 2:27 PM > To: Guzman Lugo, Fernando; felipe.contre...@gmail.com > Cc: gre...@suse.de; hiroshi.d...@nokia.com; > linux-ker...@vger.kernel.org; andy.shevche...@gmail.com; > linux-omap@vger.kernel.org; linux-arm-ker...@lists.infradead.org > Subject: RE: [PATCH 8/8] staging: tidspbridge - make > sync_wait_on_event interruptible > > fernando.l...@ti.com wrote: > > > fernando.l...@ti.com wrote: > > > > > On Tue, Oct 26, 2010 at 3:51 AM, Fernando Guzman Lugo > > > > > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/8] staging: tidspbridge - make sync_wait_on_event interruptible
fernando.l...@ti.com wrote: > > fernando.l...@ti.com wrote: > > > > On Tue, Oct 26, 2010 at 3:51 AM, Fernando Guzman Lugo > > > > 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. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/8] staging: tidspbridge - make sync_wait_on_event interruptible
> -Original Message- > From: Felipe Contreras [mailto:felipe.contre...@nokia.com] > Sent: Tuesday, October 26, 2010 12:03 PM > To: Guzman Lugo, Fernando; felipe.contre...@gmail.com > Cc: gre...@suse.de; hiroshi.d...@nokia.com; > linux-ker...@vger.kernel.org; andy.shevche...@gmail.com; > linux-omap@vger.kernel.org; linux-arm-ker...@lists.infradead.org > Subject: RE: [PATCH 8/8] staging: tidspbridge - make > sync_wait_on_event interruptible > > fernando.l...@ti.com wrote: > > > On Tue, Oct 26, 2010 at 3:51 AM, Fernando Guzman Lugo > > > 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. 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. Regards, Fernando. > > -- > Felipe Contreras > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/8] staging: tidspbridge - make sync_wait_on_event interruptible
fernando.l...@ti.com wrote: > > On Tue, Oct 26, 2010 at 3:51 AM, Fernando Guzman Lugo > > 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? -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/8] staging: tidspbridge - make sync_wait_on_event interruptible
> -Original Message- > From: Felipe Contreras [mailto:felipe.contre...@gmail.com] > Sent: Monday, October 25, 2010 7:59 PM > To: Guzman Lugo, Fernando > Cc: gre...@suse.de; felipe.contre...@nokia.com; > hiroshi.d...@nokia.com; linux-ker...@vger.kernel.org; > andy.shevche...@gmail.com; linux-omap@vger.kernel.org; > linux-arm-ker...@lists.infradead.org > Subject: Re: [PATCH 8/8] staging: tidspbridge - make > sync_wait_on_event interruptible > > On Tue, Oct 26, 2010 at 3:51 AM, Fernando Guzman Lugo > 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. Regards, Fernando. > > -- > Felipe Contreras > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] staging: tidspbridge - make sync_wait_on_event interruptible
On Tue, Oct 26, 2010 at 3:51 AM, Fernando Guzman Lugo 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. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] staging: tidspbridge - make sync_wait_on_event interruptible
So that avoid non-killable process.
Signed-off-by: Fernando Guzman Lugo
---
.../staging/tidspbridge/include/dspbridge/sync.h | 13 +++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/tidspbridge/include/dspbridge/sync.h
b/drivers/staging/tidspbridge/include/dspbridge/sync.h
index e2651e7..df05b8f 100644
--- a/drivers/staging/tidspbridge/include/dspbridge/sync.h
+++ b/drivers/staging/tidspbridge/include/dspbridge/sync.h
@@ -80,13 +80,22 @@ void sync_set_event(struct sync_object *event);
* This functios will wait until @event is set or until timeout. In case of
* success the function will return 0 and
* in case of timeout the function will return -ETIME
+ * in case of signal the function will return -ERESTARTSYS
*/
static inline int sync_wait_on_event(struct sync_object *event,
unsigned timeout)
{
- return wait_for_completion_timeout(&event->comp,
- msecs_to_jiffies(timeout)) ? 0 : -ETIME;
+ int res;
+
+ res = wait_for_completion_interruptible_timeout(&event->comp,
+ msecs_to_jiffies(timeout));
+ if (!res)
+ res = -ETIME;
+ else if (res > 0)
+ res = 0;
+
+ return res;
}
/**
--
1.6.3.3
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
