On 30 Sep, John Baldwin wrote:
> On Friday 30 September 2005 11:25 am, Antoine Pelisse wrote:
>> On 9/30/05, John Baldwin <[EMAIL PROTECTED]> wrote:
>> > On Friday 30 September 2005 05:24 am, Antoine Pelisse wrote:
>> > > Hi Robert,
>> > > I don't think your patch is correct, the total linked list can be
>> > > broken
>> > >
>> > > while the lock is released, thus just passing the link may not be
>> > > enough I have submitted a PR[1] for this a month ago but nobody took
>> > > care of it yet Regards,
>> > > Antoine Pelisse
>> > >
>> > > [1] http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/84684
>> >
>> > I think this patch looks ok. Robert, can you get the original panic on
>> > this
>> > thread tested against this patch?
>>
>>  I had a small program which could reproduce this panic in 10 seconds, it
>> was basically creating empty threads and calling kvm_getprocs() in the same
>> time. Anyway the patch was able to stop the program from panicing.
>> The panic is also reproducible in RELENG_6 and HEAD IIRC.
> 
> It turns out that the sysctl buffer is already wired in one of the two cases 
> that this function is called, so I moved the wiring up to the upper layer in 
> the other case and cut out a bunch of the locking gymnastics as a result.  
> Can you try this patch?
> 
> Index: kern_proc.c
> ===================================================================
> RCS file: /usr/cvs/src/sys/kern/kern_proc.c,v
> retrieving revision 1.231
> diff -u -r1.231 kern_proc.c
> --- kern_proc.c       27 Sep 2005 18:03:15 -0000      1.231
> +++ kern_proc.c       30 Sep 2005 17:04:57 -0000
> @@ -875,22 +875,16 @@
>  
>       if (flags & KERN_PROC_NOTHREADS) {
>               fill_kinfo_proc(p, &kinfo_proc);
> -             PROC_UNLOCK(p);
>               error = SYSCTL_OUT(req, (caddr_t)&kinfo_proc,
>                                  sizeof(kinfo_proc));
> -             PROC_LOCK(p);
>       } else {
> -             _PHOLD(p);
>               FOREACH_THREAD_IN_PROC(p, td) {
>                       fill_kinfo_thread(td, &kinfo_proc);
> -                     PROC_UNLOCK(p);
>                       error = SYSCTL_OUT(req, (caddr_t)&kinfo_proc,
>                                          sizeof(kinfo_proc));
> -                     PROC_LOCK(p);
>                       if (error)
>                               break;
>               }
> -             _PRELE(p);
>       }
>       PROC_UNLOCK(p);
>       if (error)
> @@ -932,6 +926,9 @@
>       if (oid_number == KERN_PROC_PID) {
>               if (namelen != 1) 
>                       return (EINVAL);
> +             error = sysctl_wire_old_buffer(req, 0);
> +             if (error)
> +                     return (error);         
>               p = pfind((pid_t)name[0]);
>               if (!p)
>                       return (ESRCH);
> 

sched_lock needs to be grabbed before the FOREACH_THREAD_IN_PROC loop.

Can _PHOLD()/_PRELE() be dropped?

_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to