* 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).