Date: Sun, 21 Dec 2008 23:31:18 +0100 From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@des.no> To: Ferner Cilloniz <fernercc@gmail.com> Cc: freebsd-hackers@freebsd.org, Julian Elischer <julian@elischer.org> Subject: Re: adding proc to allproc Message-ID: <86ocz55fex.fsf@ds4.des.no> In-Reply-To: <1229739800.5614.24.camel@mobiliare.Belkin> (Ferner Cilloniz's message of "Sat, 20 Dec 2008 02:23:20 %2B0000") References: <1229726360.5614.15.camel@mobiliare.Belkin> <494C8246.3020703@elischer.org> <1229729326.5614.16.camel@mobiliare.Belkin> <494C8508.2020000@elischer.org> <1229739800.5614.24.camel@mobiliare.Belkin>
next in thread | previous in thread | raw e-mail | index | archive | help
Ferner Cilloniz <fernercc@gmail.com> writes:
> The process comes from the allproc list.
> I am simply iterating through this list and removing the first 1/2 of
> allproc list and adding the removed process to my own singly linked
> list.
>
> LIST_REMOVE(current_proc, p_list); // remove from the allproc
> add_proc_entry(current_proc);
You can't do that without holding allproc_lock.
> Later, i am iterating through my singly linked list and doing the below:
>
> struct proc *p =3D current->p;
> f( p !=3D NULL && (p->p_state =3D=3D PRS_NEW || p->p_state =3D=3D PRS_NOR=
MAL) ){
> LIST_INSERT_HEAD(&allproc, p, p_list);
> }
You can't do that without holding allproc_lock.
> struct proc_ll *new_entry =3D (struct proc_ll*)malloc(sizeof(struct
> proc_ll), M_TEMP, M_ZERO | M_NOWAIT);
unnecessary cast, missing NULL check
> int done =3D 0;
unused variable
> new_entry->p =3D p; // maybe doing this assignment isnt correct?
>
> if(proc_entries =3D=3D NULL) {
> proc_entries =3D new_entry;
> return;
> }
>
> struct proc_ll *current =3D proc_entries;
>
> while(current !=3D NULL) {
> if(current->next =3D=3D NULL) {
> current->next =3D new_entry;
> break;
> }
> current =3D current->next;
> }
This is gross. The usual idiom is
new_entry->next =3D proc_entries;
proc_entries =3D new_entry;
However, the simplest solution is to use the <sys/queue.h> macros.
> A circular list does sound like it could cause hanging to occur but the
> above code, atleast from my eyes, doesn't seem to cause it.
With no locking, who knows what really goes on in your code...
DES
--=20
Dag-Erling Sm=C3=B8rgrav - des@des.no
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86ocz55fex.fsf>