Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 29 Apr 2004 22:04:18 +1000 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        John Baldwin <jhb@freebsd.org>
Cc:        Alex Lyashkov <shadow@psoft.net>
Subject:   Re: code cleanup
Message-ID:  <20040429214942.T11397@gamplex.bde.org>
In-Reply-To: <200404281541.08851.jhb@FreeBSD.org>
References:  <Pine.BSF.4.21.0404281120480.73191-100000@InterJet.elischer.org> <200404281541.08851.jhb@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 28 Apr 2004, John Baldwin wrote:

> On Wednesday 28 April 2004 02:26 pm, Julian Elischer wrote:
> > On Wed, 28 Apr 2004, John Baldwin wrote:
> > > On Wednesday 28 April 2004 02:26 am, Alex Lyashkov wrote:
> > > > Hi All
> > > >
> > > > how i see many points at kernel work with allproc list direct, but
> > > > proc.h introduce macros FOREACH_PROC_IN_SYSTEM.
> > > > This patch clean this places.
> > >
> > > I'd actually rather see the FOREACH_PROC macro removed, I don't think
> > > hiding the fact that it's a TAILQ is all that useful.

I'd rather it had never been.

> > it makes it possible (well, easier) to do:
> >
> > FOREACH_PROC_IN_SYSTEM(p) {
> > 	FOREACH_KSEGROUP_IN_PROC(p, kg) {
> > 		FOREACH_THREAD_IN_GROUP(kg.td) {
> > 			something(td, kg);
> > 		}
> > 	}
> > }
> >
> > Which is a lot easier to read and understand
> > than the expanded version. You don't kave to remember the linkage
> > pointer's names and you can add debugging  to it
> > and check that the correct loks are held etc.
> > (the latter being a major reason I did it).

This macro seemed more reasonable when it was added because its scope
was limited and I thought it was temporary.

> Note that the allproc_lock protects the allproc list.  W/o the FOREACH_PROC
> macro, I can grep for 'allproc' in the source tree to find all users to
> verify locking, etc.  With the extra macro, I now have to do multiple greps.
> When you multiple the effect with several wrapper macros, it now becomes much
> more work to work on locking the lists of structures since you have to do
> multiple greps to find the places to look at.  I think remembering the
> linkages for lists is actually quite important to avoid using the same
> linkage for multiple lists incorrectly.

Macros are bad for debugging.  The above is a sort of high level aspect
of debugging.  One low level one is when need to look at the linkages
for the lists.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040429214942.T11397>