Re: [PATCH resend] video: omap24xxcam: Fix compilation

[David Cohen]

On Sat, Feb 19, 2011 at 5:00 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> Hi,
>
> On Sat, Feb 19, 2011 at 01:35:09PM +0200, David Cohen wrote:
>> >> aha, now I get it, so shouldn't the real fix be including <linux/sched.h>
>> >> on <linux/wait.h>, I mean, it's <linuux/wait.h> who uses a symbol
>> >> defined in <linux/sched.h>, right ?
>>
>> That's a tricky situation. linux/sched.h includes indirectly
>> linux/completion.h which includes linux/wait.h.
>
> Ok, so the real problem is that there is circular dependency between
> <linux/sched.h> and <linux/wait.h>
>
>> By including sched.h in wait.h, the side effect is completion.h will
>> then include a blank wait.h file and trigger a compilation error every
>> time wait.h is included by any file.
>
> true, but the real problem is the circular dependency between those
> files.
>
>> > Surprisingly many other files still don't seem to be affected. But this
>> > is actually a better solution (to include sched.h in wait.h).
>>
>> It does not affect all files include wait.h because TASK_* macros are
>> used with #define statements only. So it has no effect unless some
>> file tries to use a macro which used TASK_*. It seems the usual on
>> kernel is to include both wait.h and sched.h when necessary.
>> IMO your patch is fine.
>
> I have to disagree. The fundamental problem is the circular dependency
> between those two files:
>
> sched.h uses wait_queue_head_t defined in wait.h
> wait.h uses TASK_* defined in sched.h
>
> So, IMO the real fix would be clear out the circular dependency. Maybe
> introducing <linux/task.h> to define those TASK_* symbols and include
> that on sched.h and wait.h
>
> Just dig a quick and dirty to try it out and works like a charm

We have 2 problems:
 - omap24xxcam compilation broken
 - circular dependency between sched.h and wait.h

To fix the broken compilation we can do what the rest of the kernel is
doing, which is to include sched.h.
Then, the circular dependency is fixed by some different approach
which would probably change *all* current usage of TASK_*.

IMO, there's no need to create a dependency between those issues.

Br,

David

>
> --
> balbi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/