From owner-freebsd-current Tue Nov 26 18: 5:11 2002 Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 15BBD37B401 for ; Tue, 26 Nov 2002 18:05:09 -0800 (PST) Received: from sccrmhc02.attbi.com (sccrmhc02.attbi.com [204.127.202.62]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7411B43ED1 for ; Tue, 26 Nov 2002 18:05:08 -0800 (PST) (envelope-from julian@elischer.org) Received: from InterJet.elischer.org (12-232-168-4.client.attbi.com[12.232.168.4]) by sccrmhc02.attbi.com (sccrmhc02) with ESMTP id <200211270205070020017fcce>; Wed, 27 Nov 2002 02:05:07 +0000 Received: from localhost (localhost.elischer.org [127.0.0.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id SAA55592; Tue, 26 Nov 2002 18:01:36 -0800 (PST) Date: Tue, 26 Nov 2002 18:01:34 -0800 (PST) From: Julian Elischer To: Terry Lambert Cc: Nate Lawson , current@FreeBSD.ORG Subject: Re: [PATCH] Searching for users of netncp and nwfs to help debug 5.0 problems In-Reply-To: <3DE406D6.B74F8FBC@mindspring.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG some comments: firstly: ncp_conn_locklist(int flags, struct proc *p) { ! struct thread *td = TAILQ_FIRST(&p->p_threads); /* XXX */ ! ! return lockmgr(&listlock, flags | LK_CANRECURSE, 0, td); } can't you use unidifs? :-) ok.... there is a Macro to find the first thread in a process. FIRST_THREAD_IN_PROC(p) I use it because it makes it easy to find the places that are DEFINITLY BROKEN. The marker for a KSE breakage is: XXXKSE, but places that use FIRST_THREAD_IN_PROC(p) are marked implicitly since nearly any time it is used, the code is trying to do something that doesn't make sense. (except in fork and maybe exit and exec, and in things like linux emulation where you KNOW there is only one thread). if you see TAILQ_FIRST(&p->p_threads) (or FIRST_THREAD_IN_PROC(p)) you can pretty much guarantee that what is happenning there need to be completely rewritten, possibly to the extent of rerwiting it's callers and the caller's callers. That is the problem I hit when trying to convert it to start with... Since I don't know the way that process IDs come into the session control (you have to understand the protocol for that) I basically hit a wall on trying to work out what to rewrite, and how. BTW the obnoxious FIRST_THREAD_IN_PROC will go away when we have got rid of most of the broken code and be replaced in places for which it is ok with p2td() which will be: __inline struct thread * p2td(struct proc *p) { KASSERT(((p->p_flag & P_KSES) == 0), ("Threaded program uses p2td()")); return (TAILQ_FIRST(&p->p_threads)); } You can always get from a thread to a single process but the reverse always presents the question "which thread?". This question can only be answered with external information or if you know there is only one thread at this moment. julian On Tue, 26 Nov 2002, Terry Lambert wrote: > Nate Lawson wrote: > > > It's not so much that I volunteered as I said that I'd help with > > > thread/proc issues.. > > > The trouble was that there are places where it used a proc in the old > > > code, but in some cases it needs to be a proc, and in other cases it now > > > needs to be a thread. But all they stored was the proc. Also, from > > > my memories of the code you needed to understand the protocol to know > > > which needed to be which, and I don't know that protocol. > > > > > > In addition whoever does it needs to remember that any structure that > > > stores a thread poitner is probably in error, as threads > > > are transient items and any stored thread pointer is probably a wild > > > pointer within a few milliseconds of being stored. :-) > > > > I'll take a whack at it and send it out by tomorrow, working or not. > > Don't bother. 8-). > > The attached patch makes it compile, and takes a shot at doing the > right thing. > > The threasd stuff is problematic; it's useful only for a blocking > context. The process stuff is there to identify the connection, > actually, which can mean huge latencies (hence the caching of > procp). > > It helps to know that the protocol is exclusively request/response > per session, the current code handles only a single session per > process (not one per thread), and that lock requests are answered > bith synchronously and asynchronously (request/response, then async > message on timeout or success). > > -- Terry To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message