adding proc to allproc

Dag-Erling Smørgrav des at des.no
Sun Dec 21 14:31:20 PST 2008


Ferner Cilloniz <fernercc at 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 = current->p;
> f( p != NULL && (p->p_state == PRS_NEW || p->p_state == PRS_NORMAL) ){
>       LIST_INSERT_HEAD(&allproc, p, p_list);
> }

You can't do that without holding allproc_lock.

>     struct proc_ll *new_entry = (struct proc_ll*)malloc(sizeof(struct
> proc_ll), M_TEMP, M_ZERO | M_NOWAIT);

unnecessary cast, missing NULL check

>     int done = 0;

unused variable

>     new_entry->p = p; // maybe doing this assignment isnt correct?
>
>     if(proc_entries == NULL) {
>         proc_entries = new_entry;
>         return;
>     }
>
>     struct proc_ll *current = proc_entries;
>
>     while(current != NULL) {
>         if(current->next == NULL) {
>             current->next = new_entry;
>             break;
>         }
>         current = current->next;
>     }

This is gross.  The usual idiom is

        new_entry->next = proc_entries;
        proc_entries = 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
-- 
Dag-Erling Smørgrav - des at des.no


More information about the freebsd-hackers mailing list