linux-kernel - Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1298306607.24121.18.camel@twins>
Date:	Mon, 21 Feb 2011 17:43:27 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	balbi@...com
Cc:	David Cohen <dacohen@...il.com>, linux-kernel@...r.kernel.org,
	mingo@...e.hu, linux-omap@...r.kernel.org,
	linux-media@...r.kernel.org, Alexey Dobriyan <adobriyan@...il.com>,
	Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH v2 1/1] headers: fix circular dependency between
 linux/sched.h and linux/wait.h

On Mon, 2011-02-21 at 18:29 +0200, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Feb 21, 2011 at 05:20:45PM +0100, Peter Zijlstra wrote:
> > > > I think Alexey already told you what you done wrong.
> > > >
> > > > Also, I really don't like the task_state.h header, it assumes a lot of
> > > > things it doesn't include itself and only works because its using macros
> > > > and not inlines at it probably should.
> > > 
> > > Like wait.h I'd say. The main issue is wait.h uses TASK_* macros but
> > > cannot properly include sched.h as it would create a circular
> > > dependency. So a file including wait.h is able to compile because the
> > > dependency of sched.h relies on wake_up*() macros and it's not always
> > > used.
> > > We can still drop everything else from task_state.h but the TASK_*
> > > macros and then the problem you just pointed out won't exist anymore.
> > > What do you think about it?
> > 
> > I'd much rather see a real cleanup.. eg. remove the need for sched.h to
> > include wait.h.
> 
> isn't that exactly what he's trying to achieve ? Moving TASK_* to its
> own header is one approach, what other approach do you suggest ?

No, he's making a bigger mess, and didn't I just make another
suggestion?

> > afaict its needed because struct signal_struct and struct sighand_struct
> > include a wait_queue_head_t. The inclusion seems to come through
> 
> yes.

Is that a qualified statement that, yes, that is the only inclusion
path?

> > completion.h, but afaict we don't actually need to include completion.h
> > because all we have is a pointer to a completion, which is perfectly
> > fine with an incomplete type.
> 
> so maybe just dropping completion.h from sched.h would do it.

No, that will result in non-compilation due to wait_queue_head_t usage.

> > This all would suggest we move the signal bits into their own header
> > (include/linux/signal.h already exists and seems inviting).
> > 
> > And then make sched.c include signal.h and completion.h.
> 
> you wouldn't prevent the underlying problem which is the need to include
> sched.h whenever you include wait.h and use wake_up*()

If you'd applied your brain for a second before hitting reply you'd have
noticed that at this point you'd (likely) be able to include sched.h
from wait.h. which is the right way about, you need to be able to
schedule in order to build waitqueues.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/