From owner-freebsd-hackers Mon Mar 1 19:13: 6 1999 Delivered-To: freebsd-hackers@freebsd.org Received: from smtp03.primenet.com (smtp03.primenet.com [206.165.6.133]) by hub.freebsd.org (Postfix) with ESMTP id 2B98C15400 for ; Mon, 1 Mar 1999 19:13:04 -0800 (PST) (envelope-from tlambert@usr01.primenet.com) Received: (from daemon@localhost) by smtp03.primenet.com (8.8.8/8.8.8) id UAA18425; Mon, 1 Mar 1999 20:12:46 -0700 (MST) Received: from usr01.primenet.com(206.165.6.201) via SMTP by smtp03.primenet.com, id smtpd018386; Mon Mar 1 20:12:37 1999 Received: (from tlambert@localhost) by usr01.primenet.com (8.8.5/8.8.5) id UAA21878; Mon, 1 Mar 1999 20:12:35 -0700 (MST) From: Terry Lambert Message-Id: <199903020312.UAA21878@usr01.primenet.com> Subject: Re: lockf and kernel threads To: jplevyak@inktomi.com (John Plevyak) Date: Tue, 2 Mar 1999 03:12:35 +0000 (GMT) Cc: tlambert@primenet.com, jplevyak@inktomi.com, hackers@FreeBSD.ORG In-Reply-To: <19990301091105.B21935@tsdev.inktomi.com> from "John Plevyak" at Mar 1, 99 09:11:05 am X-Mailer: ELM [version 2.4 PL25] MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG > Alternatively, if one can ensure that the p_leader does not > die before the peers, then one can preserve the locking code > and refraim from changing the proc strucure since one can > simply pass p->p_leader down to the locking code. Since for > unthreaded programs p == p->p_leader this yields very simple > code for both cases. I think you can do this by recursing on the peers in the middle of handling the kill before you pass it to the trampoline (then it's too late). > Moreover, this change enables other such threading problems to be fixed > because logically 'process-type' state can be stored in > p->p_leader->foo and logically 'thread-type' state can be stored > in p->foo. > > (this is the change I chose) Really, the thread specific data needs to be seperated from the proc struct; that is, the p->p_leader->foo should become meaningless. Basically, the litany on this is "everything that can differ between threads". > > POSIX is very explicit, that the first close on a file within a single > > process results in the locks being deasserted. > > > > I think that your patches fail to address this issue, since in reality, > > I believe that you want to treat each thread as if it were a seperate > > process for the purpose of specifying POSIX close semantics. > > The patch does in fact handle this case. The first closing > thread with pass p->p_leader into the VOP_ADVLOCK and cause the lock > to be released. > > In the current system, only if the original thread which obtained > the lock was the first to close it would the lock be released. I'm confused about the semantics you desire, I think. My opinion would be that the POSIX close/unlock coupling is highly undesirable in general, and that you would want the threads to compete as if they were processes for the semantics. I think this would depends on the lock scoping? E.g.: PTHREAD_PROCESS_PRIVATE PTHREAD_PROCESS_SHARED ? > I understand the problem for NFS locking, but for threaded programs > it would seem that lock shadowing would be the desired behavior > The program is logically one process, and the lock ranges are > shared state, not thread-specific state. I think the POSIX semantics, since they predate threads, can be reasonably interpreted either way. However, the use that you described seemed to want to scope file descriptors to a particular thread, such that a thread exit would result in the close. I don't think this is reasonable (the descriptor space is shared between all threads in a process). The only way to reasonably achieve such scoping is pthread_cleanup_push(). > I considered that also. There are a few problems with this: > > 1. some signals cannot be blocked. In particular, if the non-locking > thread is 'KILL'ed, then the lock will never be released in the > current code. A process that is sent a signal whose default (or uninterceptable) action is to cause the process to exit, stop, or continue *MUST* apply that signal to all threads within the process. If the other threads continue running, then you have delivered the signal not to the process (which is where it's scoped!), but to a particular thread. This is bad. > 2. the abort(3) code explicitly unblocks the calling threads signal > handler so that strategy will not work for asserts, aborts. > > 3. signals are used for other purposes, some of which are not > amenable to the 'disallow-all-but-on-thread' technique. This is a bug. I believe that this was done as a workaround for some other random signal handling change. We actually had to back the change out here, locally, when we were pursuing an ACAP implementation. From my reading of the GO SOLO 2 CDROM: signal/sighold/sigignore/sigpause/sigrelse/sigset ... Use of any of these functions is unspecified in a multi-threaded process. [ ... ] sigaction ... At the time of generation, a determination is made whether the signal has been generated for the process or for a specific thread within the process. Signals which are generated by some action attributable to a particular thread, such as a hardware fault <>, are generated for the thread that caused the signal to be generated. Signals that are generated in association with a process ID or group ID or an asynchronous event such as terminal activity are generated for the process. ... When a signal is delivered to a thread, if the action of that signal specifies termination, stop, or continue, the entire process will be terminated, stopped, or continued, respectively. The whole thing, in its entirety, is a *huge* condemnation of the current FreeBSD implementation, including the process group notification of SIGHUP not being delivered to some group members, and instead read returning 0/-1 for ttys. Terry Lambert terry@lambert.org --- Any opinions in this posting are my own and not those of my present or previous employers. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message