Date: Wed, 4 Oct 2000 09:39:31 -0700 (PDT) From: Matt Dillon <dillon@earth.backplane.com> To: Peter Wemm <peter@netplex.com.au> Cc: Chuck Paterson <cp@bsdi.com>, Alfred Perlstein <bright@wintelcom.net>, John Baldwin <jhb@FreeBSD.ORG>, arch@FreeBSD.ORG, John Polstra <jdp@polstra.com>, Daniel Eischen <eischen@vigrid.com>, Greg Lehey <grog@lemis.com> Subject: Re: Mutexes and semaphores Message-ID: <200010041639.e94GdVR24384@earth.backplane.com> References: <200010040727.e947RpH19302@netplex.com.au>
next in thread | previous in thread | raw e-mail | index | archive | help
This is one of the few things I really hate in the linux source base. Besides, it isn't guarenteed to work. What happens if the process double-reparent's and the space pointed to by the original pptr is reallocated and reused in the second reparenting? That is, this cpu stalls after getting 'parent', the process is reparented (p->p_pptr changes), the process reparents AGAIN to a new just-allocated process which happens to use the original p->p_pptr's process's zalloc slot. In this case the contents of parent->p_pid can be garbage and the algorithm will not detect it. Now, granted, the chance of the above occuring is probably close to nil. From an algorithmic point of view, though, the below code (and the linux code) cannot actually guarentee correct operation without also make assumptions on side effects (for example, in regards to who is or is not actually allowed to become the new parent and whether the situation can occur with those restrictions in place). I would prefer not to see this sort of code in FreeBSD, even if it means slowing things down a little. -Matt :In the freebsd case, this is the case. Zones are never cleaned up, and :certainly not unmapped. zfree() will however cause the first few bytes :to be clobbered as they are reused for the freelist. : :int :getppid(p, uap) : struct proc *p; : struct getppid_args *uap; :{ : : p->p_retval[0] = p->p_pptr->p_pid; : return (0); :} : :could safely become: : :int :getppid(p, uap) : struct proc *p; : struct getppid_args *uap; :{ : struct proc *parent; : pid_t pid; : : parent = p->p_pptr; : pid = parent->p_pid; :#ifdef SMP : for (;;) { : __asm __volatile (": : : memory"); /* mb(); on x86 */ : if (parent == p->p_pptr) : break; : /* lost a race, our parent died and we reparented - redo */ : parent = p->p_pptr; : pid = parent->p_pid; : } :#endif : p->p_retval[0] = (register_t)pid; : return 0; :} : :This isn't quite the same as the linux version, but I'm pretty sure the :important races are covered just like theirs. The mb(); replacement could :be a real per-cpu mb instruction on arches that require it. Even if it is :just the gcc flush, it would be sufficient for 99.99999% of cases. : :Cheers, :-Peter To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200010041639.e94GdVR24384>