2008-07-01 11:55:41

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH] drivers/input/ff-core.c needs <linux/sched.h>

commit 656acd2bbc4ce7f224de499ee255698701396c48
Author: Dmitry Torokhov <[email protected]>
Date: Thu Jun 26 11:30:02 2008 -0400

Input: fix locking in force-feedback core

The newly added event_lock spinlock in the input core disallows sleeping
and therefore using mutexes in event handlers. Convert force-feedback
core to rely on event_lock instead of mutex to protect slots allocated
for fore-feedback effects. The original mutex is still used to serialize
uploading and erasing of effects.

causes the following regression on m68k:

| linux/drivers/input/ff-core.c: In function 'input_ff_upload':
| linux/drivers/input/ff-core.c:172: error: dereferencing pointer to incomplete type
| linux/drivers/input/ff-core.c: In function 'erase_effect':
| linux/drivers/input/ff-core.c:197: error: dereferencing pointer to incomplete type
| linux/drivers/input/ff-core.c:204: error: dereferencing pointer to incomplete type
| make[4]: *** [drivers/input/ff-core.o] Error 1

As the incomplete type is `struct task_struct', including <linux/sched.h> fixes
it.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/input/ff-core.c | 1 +
1 file changed, 1 insertion(+)

--- a/drivers/input/ff-core.c
+++ b/drivers/input/ff-core.c
@@ -28,6 +28,7 @@
#include <linux/input.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/sched.h>

/*
* Check that the effect_id is a valid effect and whether the user

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


2008-07-01 12:47:36

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] drivers/input/ff-core.c needs <linux/sched.h>

On Tue, Jul 01, 2008 at 01:55:25PM +0200, Geert Uytterhoeven wrote:
> commit 656acd2bbc4ce7f224de499ee255698701396c48
> Author: Dmitry Torokhov <[email protected]>
> Date: Thu Jun 26 11:30:02 2008 -0400
>
> Input: fix locking in force-feedback core
>
> The newly added event_lock spinlock in the input core disallows sleeping
> and therefore using mutexes in event handlers. Convert force-feedback
> core to rely on event_lock instead of mutex to protect slots allocated
> for fore-feedback effects. The original mutex is still used to serialize
> uploading and erasing of effects.
>
> causes the following regression on m68k:
>
> | linux/drivers/input/ff-core.c: In function 'input_ff_upload':
> | linux/drivers/input/ff-core.c:172: error: dereferencing pointer to incomplete type
> | linux/drivers/input/ff-core.c: In function 'erase_effect':
> | linux/drivers/input/ff-core.c:197: error: dereferencing pointer to incomplete type
> | linux/drivers/input/ff-core.c:204: error: dereferencing pointer to incomplete type
> | make[4]: *** [drivers/input/ff-core.o] Error 1
>

Argh! Sorry about it.

> As the incomplete type is `struct task_struct', including <linux/sched.h> fixes
> it.

Not linux/spinlock.h? I wonder if I need to include linux/spinlock.h and
linux/mutex.h directly from linux/input.h... What is the current
policy on headers - do they need to include everything to be
functional or it is responsibility of the user?

--
Dmitry

2008-07-01 13:05:51

by Adrian Bunk

[permalink] [raw]
Subject: Re: [PATCH] drivers/input/ff-core.c needs <linux/sched.h>

On Tue, Jul 01, 2008 at 08:46:54AM -0400, Dmitry Torokhov wrote:
> On Tue, Jul 01, 2008 at 01:55:25PM +0200, Geert Uytterhoeven wrote:
> > commit 656acd2bbc4ce7f224de499ee255698701396c48
> > Author: Dmitry Torokhov <[email protected]>
> > Date: Thu Jun 26 11:30:02 2008 -0400
> >
> > Input: fix locking in force-feedback core
> >
> > The newly added event_lock spinlock in the input core disallows sleeping
> > and therefore using mutexes in event handlers. Convert force-feedback
> > core to rely on event_lock instead of mutex to protect slots allocated
> > for fore-feedback effects. The original mutex is still used to serialize
> > uploading and erasing of effects.
> >
> > causes the following regression on m68k:
> >
> > | linux/drivers/input/ff-core.c: In function 'input_ff_upload':
> > | linux/drivers/input/ff-core.c:172: error: dereferencing pointer to incomplete type
> > | linux/drivers/input/ff-core.c: In function 'erase_effect':
> > | linux/drivers/input/ff-core.c:197: error: dereferencing pointer to incomplete type
> > | linux/drivers/input/ff-core.c:204: error: dereferencing pointer to incomplete type
> > | make[4]: *** [drivers/input/ff-core.o] Error 1
> >
>
> Argh! Sorry about it.
>
> > As the incomplete type is `struct task_struct', including <linux/sched.h> fixes
> > it.
>
> Not linux/spinlock.h? I wonder if I need to include linux/spinlock.h and
> linux/mutex.h directly from linux/input.h... What is the current
> policy on headers - do they need to include everything to be
> functional or it is responsibility of the user?

Theoretically it's the responsibility of the header to include
everything it needs.

In practice we are after -rc8 and even thinking of this kind of #include
changes under include/linux/ makes me nervous - like the fact that the
ff-core.c problem occured _only_ on m68k our headers are too fragile for
expecting such changes to simply work.

Can we go with Geert's patch for 2.6.26 and if you want to fix it
properly you can send a patch for 2.6.27?

> Dmitry

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2008-07-01 13:14:36

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] drivers/input/ff-core.c needs <linux/sched.h>

On Tue, 1 Jul 2008, Adrian Bunk wrote:
> On Tue, Jul 01, 2008 at 08:46:54AM -0400, Dmitry Torokhov wrote:
> > On Tue, Jul 01, 2008 at 01:55:25PM +0200, Geert Uytterhoeven wrote:
> > > commit 656acd2bbc4ce7f224de499ee255698701396c48
> > > Author: Dmitry Torokhov <[email protected]>
> > > Date: Thu Jun 26 11:30:02 2008 -0400
> > >
> > > Input: fix locking in force-feedback core
> > >
> > > The newly added event_lock spinlock in the input core disallows sleeping
> > > and therefore using mutexes in event handlers. Convert force-feedback
> > > core to rely on event_lock instead of mutex to protect slots allocated
> > > for fore-feedback effects. The original mutex is still used to serialize
> > > uploading and erasing of effects.
> > >
> > > causes the following regression on m68k:
> > >
> > > | linux/drivers/input/ff-core.c: In function 'input_ff_upload':
> > > | linux/drivers/input/ff-core.c:172: error: dereferencing pointer to incomplete type
> > > | linux/drivers/input/ff-core.c: In function 'erase_effect':
> > > | linux/drivers/input/ff-core.c:197: error: dereferencing pointer to incomplete type
> > > | linux/drivers/input/ff-core.c:204: error: dereferencing pointer to incomplete type
> > > | make[4]: *** [drivers/input/ff-core.o] Error 1
> > >
> >
> > Argh! Sorry about it.
> >
> > > As the incomplete type is `struct task_struct', including <linux/sched.h> fixes
> > > it.
> >
> > Not linux/spinlock.h? I wonder if I need to include linux/spinlock.h and

Nope, tried that first...

> > linux/mutex.h directly from linux/input.h... What is the current
> > policy on headers - do they need to include everything to be
> > functional or it is responsibility of the user?
>
> Theoretically it's the responsibility of the header to include
> everything it needs.

Indeed. But due to Include Hell that's not always possible.

> In practice we are after -rc8 and even thinking of this kind of #include
> changes under include/linux/ makes me nervous - like the fact that the
> ff-core.c problem occured _only_ on m68k our headers are too fragile for
> expecting such changes to simply work.
>
> Can we go with Geert's patch for 2.6.26 and if you want to fix it
> properly you can send a patch for 2.6.27?

And let the proper patch boil in linux-next for a while...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2008-07-01 13:27:23

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] drivers/input/ff-core.c needs <linux/sched.h>

On Tue, Jul 01, 2008 at 04:05:17PM +0300, Adrian Bunk wrote:
> On Tue, Jul 01, 2008 at 08:46:54AM -0400, Dmitry Torokhov wrote:
> > On Tue, Jul 01, 2008 at 01:55:25PM +0200, Geert Uytterhoeven wrote:
> > > commit 656acd2bbc4ce7f224de499ee255698701396c48
> > > Author: Dmitry Torokhov <[email protected]>
> > > Date: Thu Jun 26 11:30:02 2008 -0400
> > >
> > > Input: fix locking in force-feedback core
> > >
> > > The newly added event_lock spinlock in the input core disallows sleeping
> > > and therefore using mutexes in event handlers. Convert force-feedback
> > > core to rely on event_lock instead of mutex to protect slots allocated
> > > for fore-feedback effects. The original mutex is still used to serialize
> > > uploading and erasing of effects.
> > >
> > > causes the following regression on m68k:
> > >
> > > | linux/drivers/input/ff-core.c: In function 'input_ff_upload':
> > > | linux/drivers/input/ff-core.c:172: error: dereferencing pointer to incomplete type
> > > | linux/drivers/input/ff-core.c: In function 'erase_effect':
> > > | linux/drivers/input/ff-core.c:197: error: dereferencing pointer to incomplete type
> > > | linux/drivers/input/ff-core.c:204: error: dereferencing pointer to incomplete type
> > > | make[4]: *** [drivers/input/ff-core.o] Error 1
> > >
> >
> > Argh! Sorry about it.
> >
> > > As the incomplete type is `struct task_struct', including <linux/sched.h> fixes
> > > it.
> >
> > Not linux/spinlock.h? I wonder if I need to include linux/spinlock.h and
> > linux/mutex.h directly from linux/input.h... What is the current
> > policy on headers - do they need to include everything to be
> > functional or it is responsibility of the user?
>
> Theoretically it's the responsibility of the header to include
> everything it needs.
>
> In practice we are after -rc8 and even thinking of this kind of #include
> changes under include/linux/ makes me nervous - like the fact that the
> ff-core.c problem occured _only_ on m68k our headers are too fragile for
> expecting such changes to simply work.
>
> Can we go with Geert's patch for 2.6.26 and if you want to fix it
> properly you can send a patch for 2.6.27?
>

No, no, I am totally fine with Geert's fix, I am talking about later of
course.

--
Dmitry

2008-07-02 20:43:44

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] drivers/input/ff-core.c needs <linux/sched.h>

On Tue, Jul 01, 2008 at 09:27:02AM -0400, Dmitry Torokhov wrote:
> On Tue, Jul 01, 2008 at 04:05:17PM +0300, Adrian Bunk wrote:
> >
> > Can we go with Geert's patch for 2.6.26 and if you want to fix it
> > properly you can send a patch for 2.6.27?
> >
>
> No, no, I am totally fine with Geert's fix, I am talking about later of
> course.

Linus, will you apply Geert's patch directly or you would prefer me
set up my tree for another pull?

--
Dmitry

2008-07-03 07:00:14

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] drivers/input/ff-core.c needs <linux/sched.h>



On Wed, 2 Jul 2008, Dmitry Torokhov wrote:
>
> Linus, will you apply Geert's patch directly or you would prefer me
> set up my tree for another pull?

I took it directly.

Linus