* locking in hidraw
@ 2008-11-04 14:34 Oliver Neukum
2008-11-04 20:36 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: Oliver Neukum @ 2008-11-04 14:34 UTC (permalink / raw)
To: Jiri Slaby; +Cc: linux-input
Hi,
in this code:
mutex_lock(&list->read_mutex);
if (list->head == list->tail) {
add_wait_queue(&list->hidraw->wait, &wait);
set_current_state(TASK_INTERRUPTIBLE);
it would make more sense to lock the mutex interruptable so all
tasks sleep the same way. Is this intentional?
Regards
Oliver
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: locking in hidraw 2008-11-04 14:34 locking in hidraw Oliver Neukum @ 2008-11-04 20:36 ` Jiri Slaby 2008-11-12 14:39 ` Jiri Kosina 0 siblings, 1 reply; 6+ messages in thread From: Jiri Slaby @ 2008-11-04 20:36 UTC (permalink / raw) To: Oliver Neukum; +Cc: linux-input, Jiri Kosina Ccing jikos. Oliver Neukum napsal(a): > Hi, > > in this code: > > mutex_lock(&list->read_mutex); > > if (list->head == list->tail) { > add_wait_queue(&list->hidraw->wait, &wait); > set_current_state(TASK_INTERRUPTIBLE); > > it would make more sense to lock the mutex interruptable so all > tasks sleep the same way. Is this intentional? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: locking in hidraw 2008-11-04 20:36 ` Jiri Slaby @ 2008-11-12 14:39 ` Jiri Kosina 2008-11-12 14:51 ` Oliver Neukum 0 siblings, 1 reply; 6+ messages in thread From: Jiri Kosina @ 2008-11-12 14:39 UTC (permalink / raw) To: Jiri Slaby, Oliver Neukum; +Cc: linux-input On Tue, 4 Nov 2008, Jiri Slaby wrote: > > in this code: > > > > mutex_lock(&list->read_mutex); > > > > if (list->head == list->tail) { > > add_wait_queue(&list->hidraw->wait, &wait); > > set_current_state(TASK_INTERRUPTIBLE); > > > > it would make more sense to lock the mutex interruptable so all > > tasks sleep the same way. Is this intentional? Well, we have the test for pending signals just below that code, right? -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: locking in hidraw 2008-11-12 14:39 ` Jiri Kosina @ 2008-11-12 14:51 ` Oliver Neukum 2008-12-01 10:07 ` Jiri Kosina 0 siblings, 1 reply; 6+ messages in thread From: Oliver Neukum @ 2008-11-12 14:51 UTC (permalink / raw) To: Jiri Kosina; +Cc: Jiri Slaby, linux-input Am Mittwoch, 12. November 2008 15:39:49 schrieb Jiri Kosina: > On Tue, 4 Nov 2008, Jiri Slaby wrote: > > > > in this code: > > > > > > mutex_lock(&list->read_mutex); > > > > > > if (list->head == list->tail) { > > > add_wait_queue(&list->hidraw->wait, &wait); > > > set_current_state(TASK_INTERRUPTIBLE); > > > > > > it would make more sense to lock the mutex interruptable so all > > > tasks sleep the same way. Is this intentional? > > Well, we have the test for pending signals just below that code, right? So the tasks blocking on the mutex sleep all the time until another task is finished and unlocks the mutex, only to return to user space because signals are pending. Therefore why not make the wait on the mutex interruptible, so the signals are processed right away? Regards Oliver ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: locking in hidraw 2008-11-12 14:51 ` Oliver Neukum @ 2008-12-01 10:07 ` Jiri Kosina 2008-12-08 9:08 ` Oliver Neukum 0 siblings, 1 reply; 6+ messages in thread From: Jiri Kosina @ 2008-12-01 10:07 UTC (permalink / raw) To: Oliver Neukum; +Cc: Jiri Slaby, linux-input On Wed, 12 Nov 2008, Oliver Neukum wrote: > > > > in this code: > > > > > > > > mutex_lock(&list->read_mutex); > > > > > > > > if (list->head == list->tail) { > > > > add_wait_queue(&list->hidraw->wait, &wait); > > > > set_current_state(TASK_INTERRUPTIBLE); > > > > it would make more sense to lock the mutex interruptable so all > > > > tasks sleep the same way. Is this intentional? > > Well, we have the test for pending signals just below that code, right? > So the tasks blocking on the mutex sleep all the time until another > task is finished and unlocks the mutex, only to return to user space > because signals are pending. Therefore why not make the wait on > the mutex interruptible, so the signals are processed right away? Yes, it makes sense. Could you please fold this change together with the hidraw changes you are going to push with the autosuspend patches? Thanks, -- Jiri Kosina ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: locking in hidraw 2008-12-01 10:07 ` Jiri Kosina @ 2008-12-08 9:08 ` Oliver Neukum 0 siblings, 0 replies; 6+ messages in thread From: Oliver Neukum @ 2008-12-08 9:08 UTC (permalink / raw) To: Jiri Kosina; +Cc: Jiri Slaby, linux-input Am Montag, 1. Dezember 2008 11:07:13 schrieb Jiri Kosina: > On Wed, 12 Nov 2008, Oliver Neukum wrote: > > > > > > in this code: > > > > > > > > > > mutex_lock(&list->read_mutex); > > > > > > > > > > if (list->head == list->tail) { > > > > > add_wait_queue(&list->hidraw->wait, &wait); > > > > > set_current_state(TASK_INTERRUPTIBLE); > > > > > it would make more sense to lock the mutex interruptable so all > > > > > tasks sleep the same way. Is this intentional? > > > Well, we have the test for pending signals just below that code, right? > > So the tasks blocking on the mutex sleep all the time until another > > task is finished and unlocks the mutex, only to return to user space > > because signals are pending. Therefore why not make the wait on > > the mutex interruptible, so the signals are processed right away? > > Yes, it makes sense. Could you please fold this change together with the > hidraw changes you are going to push with the autosuspend patches? On second thought, no, this would not be a good idea. Bug fixes, fixes for oddities such as this and clear enhancements, like autosuspend, should be kept apart. I'd rather send separate patches for this, what do you think? Regards Oliver ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-12-08 9:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-11-04 14:34 locking in hidraw Oliver Neukum 2008-11-04 20:36 ` Jiri Slaby 2008-11-12 14:39 ` Jiri Kosina 2008-11-12 14:51 ` Oliver Neukum 2008-12-01 10:07 ` Jiri Kosina 2008-12-08 9:08 ` Oliver Neukum
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).