From owner-freebsd-arch Sun Sep 30 1: 0:38 2001 Delivered-To: freebsd-arch@freebsd.org Received: from earth.backplane.com (earth-nat-cw.backplane.com [208.161.114.67]) by hub.freebsd.org (Postfix) with ESMTP id E45D637B408; Sun, 30 Sep 2001 01:00:36 -0700 (PDT) Received: (from dillon@localhost) by earth.backplane.com (8.11.6/8.11.2) id f8U800Z41709; Sun, 30 Sep 2001 01:00:00 -0700 (PDT) (envelope-from dillon) Date: Sun, 30 Sep 2001 01:00:00 -0700 (PDT) From: Matt Dillon Message-Id: <200109300800.f8U800Z41709@earth.backplane.com> To: Julian Elischer Cc: John Baldwin , Warner Losh , arch@FreeBSD.ORG Subject: Re: Style Wars References: <200109291802.f8TI2jd38468@earth.backplane.com> <3BB60D52.4A29F52E@elischer.org> Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG : :well what would your prefered style be? : There is no single style that adequately describes a human readable format for structures. It depends on the structure. It is best to leave it vague style.9. Even the stuff in style.9 right now in regards to structures is mostly junk. Nobody follows any of it (except maybe the guy who wrote it), and for good reason. Put a tab after the 'struct' keyword in a field definition? Only if I really wanted to make it unreadable! -Matt To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sun Sep 30 1: 3:39 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 8AB7037B406; Sun, 30 Sep 2001 01:03:34 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id SAA08823; Sun, 30 Sep 2001 18:03:30 +1000 Date: Sun, 30 Sep 2001 18:02:54 +1000 (EST) From: Bruce Evans X-X-Sender: To: John Baldwin Cc: Peter Wemm , , Warner Losh , Julian Elischer Subject: Re: Style Wars In-Reply-To: Message-ID: <20010930175137.T70218-100000@delplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Fri, 28 Sep 2001, John Baldwin wrote: > I'm think 1b) is the one most people have favored so far and it is rather close > to our existing style, so it's not that big of a change. Does anyone object to > 1b)? It basically results in the following changes: use 2 tab spaces instead > of 1 for type names, put the entire type name before the tab(s), and if the > type is too long, just use a space. > > > 1b) > > > > struct foo { > > int f_type; > > struct mtx f_lock; > > const char *f_name; > > volatile int f_int; > > u_int64_t f_64; > > const volatile char f_cv; > > TAILQ_ENTRY(foo) f_link; > > }; Yes, this is essentially just the current style with encouragement for more verboseness. It's interesting that indent(1) defaults to the too-large indent of 16 (-di16). I wouldn't want to go back to that :-). I prefer to use 1 space after the typename, but for FreeBSD I don't mind using 1 tab after short typenames. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sun Sep 30 12: 1:27 2001 Delivered-To: freebsd-arch@freebsd.org Received: from InterJet.elischer.org (c421509-a.pinol1.sfba.home.com [24.7.86.9]) by hub.freebsd.org (Postfix) with ESMTP id D9FFA37B405; Sun, 30 Sep 2001 12:01:21 -0700 (PDT) Received: from elischer.org (InterJet.elischer.org [192.168.1.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id MAA81441; Sun, 30 Sep 2001 12:46:34 -0700 (PDT) Message-ID: <3BB768B6.15C8CE36@elischer.org> Date: Sun, 30 Sep 2001 11:47:18 -0700 From: Julian Elischer X-Mailer: Mozilla 4.7 [en] (X11; U; FreeBSD 5.0-CURRENT i386) X-Accept-Language: en, hu MIME-Version: 1.0 To: Bruce Evans Cc: John Baldwin , Peter Wemm , arch@FreeBSD.ORG, Warner Losh Subject: Re: Style Wars References: <20010930175137.T70218-100000@delplex.bde.org> Content-Type: text/plain; charset=iso-8859-2 Content-Transfer-Encoding: 7bit Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Bruce Evans wrote: > > On Fri, 28 Sep 2001, John Baldwin wrote: > > > I'm think 1b) is the one most people have favored so far and it is rather close > > to our existing style, so it's not that big of a change. Does anyone object to > > 1b)? It basically results in the following changes: use 2 tab spaces instead > > of 1 for type names, put the entire type name before the tab(s), and if the > > type is too long, just use a space. > > > > > 1b) > > > > > > struct foo { > > > int f_type; > > > struct mtx f_lock; > > > const char *f_name; > > > volatile int f_int; > > > u_int64_t f_64; > > > const volatile char f_cv; > > > TAILQ_ENTRY(foo) f_link; > > > }; > > Yes, this is essentially just the current style with encouragement for > more verboseness. It's interesting that indent(1) defaults to the > too-large indent of 16 (-di16). I wouldn't want to go back to that > :-). I prefer to use 1 space after the typename, but for FreeBSD I > don't mind using 1 tab after short typenames. parse error 42.. example required... > > Bruce > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-arch" in the body of the message -- +------------------------------------+ ______ _ __ | __--_|\ Julian Elischer | \ U \/ / hard at work in | / \ julian@elischer.org +------>x USA \ a very strange | ( OZ ) \___ ___ | country ! +- X_.---._/ presently in San Francisco \_/ \\ v To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sun Sep 30 19:28:34 2001 Delivered-To: freebsd-arch@freebsd.org Received: from elvis.mu.org (elvis.mu.org [216.33.66.196]) by hub.freebsd.org (Postfix) with ESMTP id 705F937B40E; Sun, 30 Sep 2001 19:28:31 -0700 (PDT) Received: by elvis.mu.org (Postfix, from userid 1192) id 3280C81D01; Sun, 30 Sep 2001 21:28:26 -0500 (CDT) Date: Sun, 30 Sep 2001 21:28:26 -0500 From: Alfred Perlstein To: arch@freebsd.org Cc: iedowse@freebsd.org, wpaul@freebsd.org Subject: RPC over unix domain socket issue. Message-ID: <20010930212826.O59854@elvis.mu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG I recently did some work for my employer using FreeBSD+Sun-RPC over a unix domain socket. One of the problems I had was that large responses would fail, the reason being that sendmsg would return EMSGSIZE for transfers that were too big. Basically the workaround for me was to chunk up the responces to smaller PIPEBUF writes. I know PIPEBUF is incorrect, but using getsockopt to fetch SNDBUF returns a value that when used will also EMSGSIZE me. So first off, FreeBSD broken for large RPC responses over unix domain sockets, second, what's the correct thing to do here to avoid the EMSGSIZE without using a bogus very small send size? Is there some interface I need to use to get the max msg size to use? One other thing I noticed was that the client code simply discards any ceredentials it reads, and the server sends them anyway. I was able to change the server to client communication to use simple read/write syscalls and it worked just dandy. Any reason for not doing this? I understand why we might want to check the credentials of the server, but currently the RPC over unix domain sockets doesn't do this, a workaround (if you don't trust the server) is to have your client rpc tell the server to generate a callback to you where you can then check the credentials. So basically we can fix it by either figuring out the max message size and chunking our responces, or we can just switch to using read/write for server->client responces, although that doesn't fix the possiblity of a large request, however that's unlikely unless you're passing large strings around... These bugs are both in the old RPC and the newer tiprc. Any suggestions? I suspect that we may have to fix most of the transport layers to avoid EMSGSIZE not just the unix domain socket transport, but I wanted to get suggestions from the experts. -- -Alfred Perlstein [alfred@freebsd.org] 'Instead of asking why a piece of software is using "1970s technology," start asking why software is ignoring 30 years of accumulated wisdom.' To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 12:32:26 2001 Delivered-To: freebsd-arch@freebsd.org Received: from franklin.physics.purdue.edu (franklin.physics.purdue.edu [128.210.146.222]) by hub.freebsd.org (Postfix) with ESMTP id 6CAF037B40A for ; Mon, 1 Oct 2001 12:32:24 -0700 (PDT) Received: from physics.purdue.edu (curie.physics.purdue.edu [128.210.68.223]) by franklin.physics.purdue.edu (Postfix) with ESMTP id B708C20F0F for ; Mon, 1 Oct 2001 14:32:32 -0500 (EST) Received: by physics.purdue.edu (Postfix, from userid 12409) id 4C3EE85; Mon, 1 Oct 2001 14:32:18 -0500 (EST) Date: Mon, 1 Oct 2001 14:32:18 -0500 From: Will Andrews To: arch@FreeBSD.org Subject: Re: nfsd and mountd in the wrong place Message-ID: <20011001143218.W40556@curie.physics.purdue.edu> Reply-To: Will Andrews Mail-Followup-To: arch@FreeBSD.org References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.3.17i In-Reply-To: ; from gordont@gnf.org on Sat, Sep 29, 2001 at 01:03:35PM -0700 X-Operating-System: FreeBSD 4.3-STABLE i386 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Sat, Sep 29, 2001 at 01:03:35PM -0700, Gordon Tetlow (gordont@gnf.org) wrote: > nfsd and mountd are in /sbin > rpcbind/portmap is in /usr/sbin > > nfsd and mountd aren't useful without rpcbind/portmap. And when was the > last time you needed nfsd and mountd to boot your system? I just checked, > NetBSD has already moved nfsd and mountd to /usr/sbin. > > Is there any reason why nfsd or mountd shouldn't be moved to /usr/sbin? Well, no. But rpcbind and portmap definitely need to be moved to /sbin if FreeBSD wants to be used as a NFS diskless client, since unless I'm mistaken these are required to do NFS mounts. However, neither nfsd nor mountd are required for that activity. -- wca To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 12:40:16 2001 Delivered-To: freebsd-arch@freebsd.org Received: from earth.backplane.com (earth-nat-cw.backplane.com [208.161.114.67]) by hub.freebsd.org (Postfix) with ESMTP id 69DEC37B40B for ; Mon, 1 Oct 2001 12:40:14 -0700 (PDT) Received: (from dillon@localhost) by earth.backplane.com (8.11.6/8.11.2) id f91Je9p52325; Mon, 1 Oct 2001 12:40:09 -0700 (PDT) (envelope-from dillon) Date: Mon, 1 Oct 2001 12:40:09 -0700 (PDT) From: Matt Dillon Message-Id: <200110011940.f91Je9p52325@earth.backplane.com> To: Will Andrews Cc: arch@FreeBSD.ORG Subject: Re: nfsd and mountd in the wrong place References: <20011001143218.W40556@curie.physics.purdue.edu> Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG :On Sat, Sep 29, 2001 at 01:03:35PM -0700, Gordon Tetlow (gordont@gnf.org) wrote: :> nfsd and mountd are in /sbin :> rpcbind/portmap is in /usr/sbin :> :> nfsd and mountd aren't useful without rpcbind/portmap. And when was the :> last time you needed nfsd and mountd to boot your system? I just checked, :> NetBSD has already moved nfsd and mountd to /usr/sbin. :> :> Is there any reason why nfsd or mountd shouldn't be moved to /usr/sbin? : :Well, no. But rpcbind and portmap definitely need to be moved to :/sbin if FreeBSD wants to be used as a NFS diskless client, since :unless I'm mistaken these are required to do NFS mounts. :However, neither nfsd nor mountd are required for that activity. : :-- :wca You do not need to run portmap/rpcbind as an NFS client. You do not need to run anything, you just need a live network. -Matt To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 12:41:50 2001 Delivered-To: freebsd-arch@freebsd.org Received: from franklin.physics.purdue.edu (franklin.physics.purdue.edu [128.210.146.222]) by hub.freebsd.org (Postfix) with ESMTP id 11A8A37B40C for ; Mon, 1 Oct 2001 12:41:48 -0700 (PDT) Received: from physics.purdue.edu (curie.physics.purdue.edu [128.210.68.223]) by franklin.physics.purdue.edu (Postfix) with ESMTP id B6CE220F14 for ; Mon, 1 Oct 2001 14:41:59 -0500 (EST) Received: by physics.purdue.edu (Postfix, from userid 12409) id 74F0185; Mon, 1 Oct 2001 14:41:45 -0500 (EST) Date: Mon, 1 Oct 2001 14:41:45 -0500 From: Will Andrews To: arch@FreeBSD.ORG Subject: Re: nfsd and mountd in the wrong place Message-ID: <20011001144145.X40556@curie.physics.purdue.edu> Reply-To: Will Andrews Mail-Followup-To: arch@FreeBSD.ORG References: <20011001143218.W40556@curie.physics.purdue.edu> <200110011940.f91Je9p52325@earth.backplane.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.3.17i In-Reply-To: <200110011940.f91Je9p52325@earth.backplane.com>; from dillon@earth.backplane.com on Mon, Oct 01, 2001 at 12:40:09PM -0700 X-Operating-System: FreeBSD 4.3-STABLE i386 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Mon, Oct 01, 2001 at 12:40:09PM -0700, Matt Dillon (dillon@earth.backplane.com) wrote: > You do not need to run portmap/rpcbind as an NFS client. You do > not need to run anything, you just need a live network. Well, in that case, I'm with Gordon on that nfsd and mountd both need to be in /usr/sbin instead of /sbin. -- wca To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 12:57:40 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mail.imp.ch (mail.imp.ch [157.161.1.2]) by hub.freebsd.org (Postfix) with ESMTP id 4F12B37B401; Mon, 1 Oct 2001 12:57:33 -0700 (PDT) Received: from levais.imp.ch (levais.imp.ch [157.161.4.66]) by mail.imp.ch (8.11.1/8.11.1) with ESMTP id f91JvQa52185; Mon, 1 Oct 2001 21:57:30 +0200 (CEST) (envelope-from Martin.Blapp@imp.ch) Date: Mon, 1 Oct 2001 21:57:59 +0200 (CEST) From: Martin Blapp To: Alfred Perlstein Cc: , , Subject: Re: RPC over unix domain socket issue Message-ID: <20011001214516.V10086-100000@levais.imp.ch> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Hi Alfred, > One other thing I noticed was that the client code simply discards > any ceredentials it reads, and the server sends them anyway. I > was able to change the server to client communication to use simple > read/write syscalls and it worked just dandy. Any reason for not > doing this? I understand why we might want to check the credentials > of the server, but currently the RPC over unix domain sockets > doesn't do this, a workaround (if you don't trust the server) is > to have your client rpc tell the server to generate a callback to > you where you can then check the credentials. Afaik I know, and after some talk with Bill, the creds get only filled out from client -> server. It is not yet implemented for server -> client. If you read the code carefully, you will see this. Make a callback is a good idea for a workaround. Maybe we should note this in the docs ? Also, at the moment we have only "tpi_cots_ord" available for creds, not the connectionless transport "tpi_clts". Would this help you for you application ? We would have to add there creds too ... Beside this. There are about 5-6 little problems with the CURRENT code and UNIX-Domains. I have some commented patches available for this: They are in: http://home.teleport.ch/freebsd/userland/ Ian will look at them and commit them sometime, when he has more time again. But in the meantime they may help you :-) I'm open for changes and fixes. Martin Martin Blapp, mb@imp.ch ------------------------------------------------------------------ Improware AG, UNIX solution and service provider Zurlindenstrasse 29, 4133 Pratteln, Switzerland Phone: +41 061 826 93 00: +41 61 826 93 01 PGP Fingerprint: 57E 7CCD 2769 E7AC C5FA DF2C 19C6 DCD1 1B3A EC9C ------------------------------------------------------------------ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 14:42:40 2001 Delivered-To: freebsd-arch@freebsd.org Received: from dragon.nuxi.com (trang.nuxi.com [66.92.13.169]) by hub.freebsd.org (Postfix) with ESMTP id 5D64A37B40C for ; Mon, 1 Oct 2001 14:42:37 -0700 (PDT) Received: (from obrien@localhost) by dragon.nuxi.com (8.11.6/8.11.1) id f91LgZV98075; Mon, 1 Oct 2001 14:42:35 -0700 (PDT) (envelope-from obrien) Date: Mon, 1 Oct 2001 14:42:35 -0700 From: "David O'Brien" To: Julian Elischer Cc: arch@freebsd.org Subject: Re: KSE next steps... Message-ID: <20011001144235.B97970@dragon.nuxi.com> Reply-To: obrien@freebsd.org References: <3BB410B3.A2527C09@elischer.org> <3BB4259D.4AF78B4@elischer.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <3BB4259D.4AF78B4@elischer.org>; from julian@elischer.org on Fri, Sep 28, 2001 at 12:24:13AM -0700 X-Operating-System: FreeBSD 5.0-CURRENT Organization: The NUXI BSD group X-Pgp-Rsa-Fingerprint: B7 4D 3E E9 11 39 5F A3 90 76 5D 69 58 D9 98 7A X-Pgp-Rsa-Keyid: 1024/34F9F9D5 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Fri, Sep 28, 2001 at 12:24:13AM -0700, Julian Elischer wrote: > /* Need one of these per KSE. */ > struct ks_mailbox { What does "ks_" stand for? > int new_kse(struct ks_mailbox *mbox, int new_group); /* add a new KSE */ > /* maybe in a new kse group */ Does this replace kse_init() in the paper? > maybe reversing the sycall names... > kse_new(), kse_yield(), kse_wake(), kse_exit(), thread_abort() That would seem to better follow the convention of the paper... -- -- David (obrien@FreeBSD.org) To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 16:19:54 2001 Delivered-To: freebsd-arch@freebsd.org Received: from InterJet.elischer.org (c421509-a.pinol1.sfba.home.com [24.7.86.9]) by hub.freebsd.org (Postfix) with ESMTP id DA1D637B40E; Mon, 1 Oct 2001 16:19:50 -0700 (PDT) Received: from localhost (localhost.elischer.org [127.0.0.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id RAA88311; Mon, 1 Oct 2001 17:14:01 -0700 (PDT) Date: Mon, 1 Oct 2001 17:14:00 -0700 (PDT) From: Julian Elischer To: "David O'Brien" Cc: arch@freebsd.org Subject: Re: KSE next steps... In-Reply-To: <20011001144235.B97970@dragon.nuxi.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Mon, 1 Oct 2001, David O'Brien wrote: > On Fri, Sep 28, 2001 at 12:24:13AM -0700, Julian Elischer wrote: > > /* Need one of these per KSE. */ > > struct ks_mailbox { > > What does "ks_" stand for? basically kse, but 'kse' structures are internel to the kernel. I'm toying with calling it kse_mailbox. > > > > int new_kse(struct ks_mailbox *mbox, int new_group); /* add a new KSE */ > > /* maybe in a new kse group */ > > Does this replace kse_init() in the paper? basically thre are a number of things that need to be done to get a process off and running in KSE mode. I'm leaning towards packaging them in a slightly different set of syscalls to thiose in the paper, even though all the same steps need to be done by the time that we're finished.. > > > maybe reversing the sycall names... > > kse_new(), kse_yield(), kse_wake(), kse_exit(), thread_abort() > > That would seem to better follow the convention of the paper... yes.. > > -- > -- David (obrien@FreeBSD.org) > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 19:46:14 2001 Delivered-To: freebsd-arch@freebsd.org Received: from silby.com (cb34181-a.mdsn1.wi.home.com [24.14.173.39]) by hub.freebsd.org (Postfix) with ESMTP id 61CEF37B407 for ; Mon, 1 Oct 2001 19:46:11 -0700 (PDT) Received: (qmail 98937 invoked by uid 1000); 2 Oct 2001 02:46:09 -0000 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 2 Oct 2001 02:46:09 -0000 Date: Mon, 1 Oct 2001 21:46:09 -0500 (CDT) From: Mike Silbersack To: Subject: Reading physical memory in a cross-platform way Message-ID: <20011001214234.W98394-100000@achilles.silby.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Right now I'm working on autoscaling files/procs/etc. It's an easy job, except for one problem I keep bumping my head into: figuring out how much memory the system has. It appears that for each architecture we store the size of physical memory in a different way, requiring conversion functions that are different for each architecture. This makes my job difficult. :) So, two questions: 1. Is there a variable / function which contains the size of memory across all platforms that I am missing? 2. If not, is there a problem if I add a u_int64_t containing the size of physical memory in bytes in machdep.c for each architecture? Thanks, Mike "Silby" Silbersack To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 20:25: 7 2001 Delivered-To: freebsd-arch@freebsd.org Received: from dan.emsphone.com (dan.emsphone.com [199.67.51.101]) by hub.freebsd.org (Postfix) with ESMTP id C555B37B401 for ; Mon, 1 Oct 2001 20:25:04 -0700 (PDT) Received: (from dan@localhost) by dan.emsphone.com (8.11.4/8.11.4) id f923Oqq24491; Mon, 1 Oct 2001 22:24:52 -0500 (CDT) (envelope-from dan) Date: Mon, 1 Oct 2001 22:24:51 -0500 From: Dan Nelson To: Mike Silbersack Cc: freebsd-arch@FreeBSD.ORG Subject: Re: Reading physical memory in a cross-platform way Message-ID: <20011001222450.A39302@dan.emsphone.com> References: <20011001214234.W98394-100000@achilles.silby.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20011001214234.W98394-100000@achilles.silby.com> User-Agent: Mutt/1.3.22.1i X-OS: FreeBSD 5.0-CURRENT X-message-flag: Outlook Error Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG In the last episode (Oct 01), Mike Silbersack said: > Right now I'm working on autoscaling files/procs/etc. It's an easy > job, except for one problem I keep bumping my head into: figuring out > how much memory the system has. It appears that for each > architecture we store the size of physical memory in a different way, > requiring conversion functions that are different for each > architecture. This makes my job difficult. :) > > So, two questions: > > 1. Is there a variable / function which contains the size of memory > across all platforms that I am missing? > > 2. If not, is there a problem if I add a u_int64_t containing the size of > physical memory in bytes in machdep.c for each architecture? It looks like 'physmem' (aka the hw.physmem sysctl) is defined the same way on all the systems; are you looking for something else? -- Dan Nelson dnelson@allantgroup.com To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 20:26:53 2001 Delivered-To: freebsd-arch@freebsd.org Received: from silby.com (cb34181-a.mdsn1.wi.home.com [24.14.173.39]) by hub.freebsd.org (Postfix) with ESMTP id C44DB37B401 for ; Mon, 1 Oct 2001 20:26:50 -0700 (PDT) Received: (qmail 14371 invoked by uid 1000); 2 Oct 2001 03:26:48 -0000 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 2 Oct 2001 03:26:48 -0000 Date: Mon, 1 Oct 2001 22:26:48 -0500 (CDT) From: Mike Silbersack To: Dan Nelson Cc: Subject: Re: Reading physical memory in a cross-platform way In-Reply-To: <20011001222450.A39302@dan.emsphone.com> Message-ID: <20011001222555.S7118-100000@achilles.silby.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Mon, 1 Oct 2001, Dan Nelson wrote: > It looks like 'physmem' (aka the hw.physmem sysctl) is defined the same > way on all the systems; are you looking for something else? > > -- > Dan Nelson > dnelson@allantgroup.com physmem != hw.physmem. The sysctl is actually the output of a function, not a simple int or the like. Mike "Silby" Silbersack To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Mon Oct 1 20:46:14 2001 Delivered-To: freebsd-arch@freebsd.org Received: from dan.emsphone.com (dan.emsphone.com [199.67.51.101]) by hub.freebsd.org (Postfix) with ESMTP id ACA5937B405 for ; Mon, 1 Oct 2001 20:46:10 -0700 (PDT) Received: (from dan@localhost) by dan.emsphone.com (8.11.4/8.11.4) id f923k4574676; Mon, 1 Oct 2001 22:46:05 -0500 (CDT) (envelope-from dan) Date: Mon, 1 Oct 2001 22:46:04 -0500 From: Dan Nelson To: Mike Silbersack Cc: freebsd-arch@FreeBSD.ORG Subject: Re: Reading physical memory in a cross-platform way Message-ID: <20011001224604.A29573@dan.emsphone.com> References: <20011001222450.A39302@dan.emsphone.com> <20011001222555.S7118-100000@achilles.silby.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20011001222555.S7118-100000@achilles.silby.com> User-Agent: Mutt/1.3.22.1i X-OS: FreeBSD 5.0-CURRENT X-message-flag: Outlook Error Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG In the last episode (Oct 01), Mike Silbersack said: > On Mon, 1 Oct 2001, Dan Nelson wrote: > > It looks like 'physmem' (aka the hw.physmem sysctl) is defined the > > same way on all the systems; are you looking for something else? > > > > -- > > Dan Nelson > > dnelson@allantgroup.com > > physmem != hw.physmem. The sysctl is actually the output of a function, > not a simple int or the like. Yeah, but look at the function (i386 as an example): static int sysctl_hw_physmem(SYSCTL_HANDLER_ARGS) { int error = sysctl_handle_int(oidp, 0, ctob(physmem), req); return (error); } So the physmem value is just in pages; even though the different platforms use ctob() or _ptob(), the macros all expand to ((x) << PAGE_SHIFT). Junior Kernel Hacker project: convert alpha and ia64 to use ctob() in the name of consistency :) -- Dan Nelson dnelson@allantgroup.com To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 1:20:43 2001 Delivered-To: freebsd-arch@freebsd.org Received: from arb.arb.za.net (arb.arb.za.net [196.7.148.4]) by hub.freebsd.org (Postfix) with ESMTP id 72B3F37B408 for ; Tue, 2 Oct 2001 01:20:34 -0700 (PDT) Received: (from uucp@localhost) by arb.arb.za.net (8.11.3/8.11.3) with UUCP id f928KW852550 for arch@freebsd.org; Tue, 2 Oct 2001 10:20:32 +0200 (SAST) (envelope-from mark@grondar.za) Received: from grondar.za (localhost [127.0.0.1]) by grimreaper.grondar.za (8.11.6/8.11.6) with ESMTP id f91Gmgt25779 for ; Mon, 1 Oct 2001 17:48:42 +0100 (BST) (envelope-from mark@grondar.za) Message-Id: <200110011648.f91Gmgt25779@grimreaper.grondar.za> To: arch@freebsd.org Subject: [Patch] style.9 nit Date: Mon, 01 Oct 2001 17:48:42 +0100 From: Mark Murray Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Hi Any objections to this patch? It removes argument names from an example function prototype to match a previously mentioned guideline about such names. eg: int foo(int bar); becomes int foo(int); This helps quite a bit in reducing namespace pollution. M Index: style.9 =================================================================== RCS file: /home/ncvs/src/share/man/man9/style.9,v retrieving revision 1.66 diff -u -d -r1.66 style.9 --- style.9 1 Oct 2001 16:13:59 -0000 1.66 +++ style.9 1 Oct 2001 16:39:38 -0000 @@ -252,8 +252,7 @@ Prototypes may have an extra space after a tab to enable function names to line up: .Bd -literal -static char *function(int _arg, const char *_arg2, struct foo *_arg3, - struct bar *_arg4); +static char *function(int, const char *, struct foo *, struct bar *); static void usage(void); /* To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 1:52:58 2001 Delivered-To: freebsd-arch@freebsd.org Received: from whale.sunbay.crimea.ua (whale.sunbay.crimea.ua [212.110.138.65]) by hub.freebsd.org (Postfix) with ESMTP id 0548737B40C for ; Tue, 2 Oct 2001 01:50:40 -0700 (PDT) Received: (from ru@localhost) by whale.sunbay.crimea.ua (8.11.6/8.11.2) id f928nbK89494; Tue, 2 Oct 2001 11:49:37 +0300 (EEST) (envelope-from ru) Date: Tue, 2 Oct 2001 11:49:36 +0300 From: Ruslan Ermilov To: Mark Murray Cc: arch@FreeBSD.ORG Subject: Re: [Patch] style.9 nit Message-ID: <20011002114936.L74839@sunbay.com> References: <200110011648.f91Gmgt25779@grimreaper.grondar.za> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <200110011648.f91Gmgt25779@grimreaper.grondar.za>; from mark@grondar.za on Mon, Oct 01, 2001 at 05:48:42PM +0100 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Mon, Oct 01, 2001 at 05:48:42PM +0100, Mark Murray wrote: > Hi > > Any objections to this patch? > > It removes argument names from an example function prototype > to match a previously mentioned guideline about such names. > > eg: > > int foo(int bar); > > becomes > > int foo(int); > Hmm, what's wrong with this? : In header files visible to userland applications, prototypes that are : visible must use either protected names or no names with the types. It : is preferable to use protected names. E.g., use: : : void function(int); : : or: : : void function(int _fd); Your diff actually changes: int foo(int _bar); to int foo(int); which is acceptable prototype with "protected" names. Cheers, -- Ruslan Ermilov Oracle Developer/DBA, ru@sunbay.com Sunbay Software AG, ru@FreeBSD.org FreeBSD committer, +380.652.512.251 Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 5: 0:26 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 7E9C237B401; Tue, 2 Oct 2001 05:00:21 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id WAA27100; Tue, 2 Oct 2001 22:00:16 +1000 Date: Tue, 2 Oct 2001 21:59:38 +1000 (EST) From: Bruce Evans X-X-Sender: To: Ruslan Ermilov Cc: Mark Murray , Subject: Re: [Patch] style.9 nit In-Reply-To: <20011002114936.L74839@sunbay.com> Message-ID: <20011002212531.O87836-100000@delplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Tue, 2 Oct 2001, Ruslan Ermilov wrote: > On Mon, Oct 01, 2001 at 05:48:42PM +0100, Mark Murray wrote: > > Hi > > > > Any objections to this patch? Yes. > > It removes argument names from an example function prototype > > to match a previously mentioned guideline about such names. > > > > eg: > > > > int foo(int bar); > > > > becomes > > > > int foo(int); > > > Hmm, what's wrong with this? > > : In header files visible to userland applications, prototypes that are > : visible must use either protected names or no names with the types. It > : is preferable to use protected names. E.g., use: My version of style.9 clarifies this a little: %%% Index: style.9 =================================================================== RCS file: /home/ncvs/src/share/man/man9/style.9,v retrieving revision 1.66 diff -u -2 -r1.66 style.9 --- style.9 1 Oct 2001 16:13:59 -0000 1.66 +++ style.9 2 Oct 2001 10:59:56 -0000 @@ -238,5 +293,7 @@ .Pp In header files visible to userland applications, prototypes that are -visible must use either protected names or no names with the types. It +visible must use either +.Dq Li protected +names (ones beginning with an underscore) or no names with the types. It is preferable to use protected names. E.g., use: %%% > : > : void function(int); > : > : or: > : > : void function(int _fd); > > Your diff actually changes: > > int foo(int _bar); > > to > > int foo(int); > > which is acceptable prototype with "protected" names. Except this changes from the preferred form to the unpreferred one. The diff is actually a little different: > > -static char *function(int _arg, const char *_arg2, struct foo *_arg3, > > - struct bar *_arg4); > > +static char *function(int, const char *, struct foo *, struct bar *); Since the function is static, this example is bad for other reasons. Namespace pollution is almost irrelevant. The function proper must have certain arg names and those should normally not begin with an underscore or shadow any more global names. The function prototype can use the same names. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 7:12:11 2001 Delivered-To: freebsd-arch@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 4850F37B406 for ; Tue, 2 Oct 2001 07:12:06 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.11.6/8.11.5) with SMTP id f92EBpB98611 for ; Tue, 2 Oct 2001 10:11:51 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Tue, 2 Oct 2001 10:11:51 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: arch@FreeBSD.org Subject: nfs_lock.c and rpc.lockd: request for enlightenment on apparent evil Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG As part of the on-going capabilities development work, all kernel access control checks are being reviewed for correctness and consistency. Right now, there's some un-pretty code in the new NFS locking code: /* * XXX Hack to temporarily allow this process (regardless of it's creds) * to open the fifo we need to write to. vn_open() really should * take a ucred (and once it does, this code should be fixed to use * proc0's ucred. */ saved_uid = p->p_ucred->cr_uid; p->p_ucred->cr_uid = 0; /* temporarly run the vn_open as root */ That's pretty ugly (especially because that ucred is shared). As the comment indicates, that's probably relatively fixable -- either we can make vn_open.c use a more appropriate credential (perhaps cred0, perhaps one cached from the source of the fifo, or perhaps even keeping the fifo open so that it doesn't have to be reopened -- we can find something). But it gets worse: /* Let root, or someone who once was root (lockd generally * switches to the daemon uid once it is done setting up) make * this call. * * XXX This authorization check is probably not right. */ if ((error = suser(p)) != 0 && p->p_ucred->cr_svuid != 0) return (error); It should be noted that this is probably entirely the wrong approach to the problem. In general, only the effective uid should be used for authorization. If the process has an svuid set to something other than its ruid or euid, it can always swap them around -- probably rpc.lockd should simply set its effective uid back to 0 when it wants to perform this action, rather than remaining using daemon_uid. There are certainly advantages to running as 'daemon', including reducing the risk of using root privilege in an un-anticipated way. However, if the goal was to prevent a compromise of rpc.lockd from resulting in root compromise, that fails, since the saved uid is set (and relied on :-). However, I'm not very familiar with the implementation here, and am only just starting to get a grasp on it. It would be much faster for someone whos familiar with it (Alfred?) to look at my comments and tell me if they're off-mark. Specifically, is the following proposal going to cause problems due to other use of credentials: Remove the above svuid clause, relying purely on suser() (or in the capabilities case, appropriate privilege), and modify rpc.lockd to restore the saved uid using seteuid(), and then restore the daemon uid using setuid() on either side of the call. For the other case above, I have do do a bit more reading before suggesting a course of action, but in general, setting root privileges on a ucred to perform an action in this way is a bad idea. It's even worse because it's a shared ucred, which sets root privilege for all other processes sharing the ucred for the duration of the call, as well as any open files with that ucred cached. (people really need to figure out that ucred's are shared, or just learn not to touch ucreds) Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 10:21:56 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mail.rpi.edu (mail.rpi.edu [128.113.22.40]) by hub.freebsd.org (Postfix) with ESMTP id A7C9C37B407; Tue, 2 Oct 2001 10:21:53 -0700 (PDT) Received: from [128.113.24.47] (gilead.acs.rpi.edu [128.113.24.47]) by mail.rpi.edu (8.11.3/8.11.3) with ESMTP id f92HLOk139552; Tue, 2 Oct 2001 13:21:24 -0400 Mime-Version: 1.0 X-Sender: drosih@mail.rpi.edu Message-Id: In-Reply-To: <20011002212531.O87836-100000@delplex.bde.org> References: <20011002212531.O87836-100000@delplex.bde.org> Date: Tue, 2 Oct 2001 13:21:21 -0400 To: Bruce Evans , Ruslan Ermilov From: Garance A Drosihn Subject: Re: [Patch] style.9 nit Cc: Mark Murray , Content-Type: text/plain; charset="us-ascii" ; format="flowed" Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG At 9:59 PM +1000 10/2/01, Bruce Evans wrote: > >My version of style.9 clarifies this a little: > >%%% >Index: style.9 >=================================================================== >RCS file: /home/ncvs/src/share/man/man9/style.9,v >retrieving revision 1.66 >diff -u -2 -r1.66 style.9 >--- style.9 1 Oct 2001 16:13:59 -0000 1.66 >+++ style.9 2 Oct 2001 10:59:56 -0000 >@@ -238,5 +293,7 @@ > .Pp > In header files visible to userland applications, prototypes that are >-visible must use either protected names or no names with the types. It >+visible must use either >+.Dq Li protected >+names (ones beginning with an underscore) or no names with the types. It > is preferable to use protected names. > E.g., use: >%%% This seems like a good clarification to make to the standard style(9). >The diff is actually a little different: > >> > -static char *function(int _arg, const char *_arg2, struct >>foo *_arg3, >> > - struct bar *_arg4); >> > +static char *function(int, const char *, struct foo *, >>struct bar *); > >Since the function is static, this example is bad for other reasons. >Namespace pollution is almost irrelevant. The function proper must have >certain arg names and those should normally not begin with an underscore >or shadow any more global names. The function prototype can use the >same names. For static routines (ones in a *.c source file), should any prototypes at the top of the file include the exact argument names, without the underscore? Are the protected names only advisable for prototypes in include files? -- Garance Alistair Drosehn = gad@eclipse.acs.rpi.edu Senior Systems Programmer or gad@freebsd.org Rensselaer Polytechnic Institute or drosih@rpi.edu To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 10:27:16 2001 Delivered-To: freebsd-arch@freebsd.org Received: from relay.gnf.org (relay.gnf.org [208.44.31.36]) by hub.freebsd.org (Postfix) with ESMTP id F033037B407 for ; Tue, 2 Oct 2001 10:27:13 -0700 (PDT) Received: from mail.gnf.org (smtp.gnf.org [10.0.0.11]) by relay.gnf.org (8.11.6/8.11.6) with ESMTP id f92HRDj15035 for ; Tue, 2 Oct 2001 10:27:13 -0700 Received: by mail.gnf.org (Postfix, from userid 888) id 2BF5D11E505; Tue, 2 Oct 2001 10:26:26 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by mail.gnf.org (Postfix) with ESMTP id 2443511A572 for ; Tue, 2 Oct 2001 10:26:26 -0700 (PDT) Date: Tue, 2 Oct 2001 10:26:26 -0700 (PDT) From: Gordon Tetlow To: Subject: Re: nfsd and mountd in the wrong place In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG I submitted a pr on this issue... http://www.freebsd.org/cgi/query-pr.cgi?pr=30972 -gordon On Sat, 29 Sep 2001, Gordon Tetlow wrote: > nfsd and mountd are in /sbin > rpcbind/portmap is in /usr/sbin > > nfsd and mountd aren't useful without rpcbind/portmap. And when was the > last time you needed nfsd and mountd to boot your system? I just checked, > NetBSD has already moved nfsd and mountd to /usr/sbin. > > Is there any reason why nfsd or mountd shouldn't be moved to /usr/sbin? > > -gordon > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-arch" in the body of the message > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 10:29:42 2001 Delivered-To: freebsd-arch@freebsd.org Received: from silby.com (cb34181-a.mdsn1.wi.home.com [24.14.173.39]) by hub.freebsd.org (Postfix) with ESMTP id BF7E037B407 for ; Tue, 2 Oct 2001 10:29:36 -0700 (PDT) Received: (qmail 16718 invoked by uid 1000); 2 Oct 2001 17:29:14 -0000 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 2 Oct 2001 17:29:14 -0000 Date: Tue, 2 Oct 2001 12:29:14 -0500 (CDT) From: Mike Silbersack To: Dan Nelson Cc: Subject: Re: Reading physical memory in a cross-platform way In-Reply-To: <20011001224604.A29573@dan.emsphone.com> Message-ID: <20011002122651.P16126-200000@achilles.silby.com> MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="0-1762315182-1002043754=:16126" Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. Send mail to mime@docserver.cac.washington.edu for more info. --0-1762315182-1002043754=:16126 Content-Type: TEXT/PLAIN; charset=US-ASCII On Mon, 1 Oct 2001, Dan Nelson wrote: > > physmem != hw.physmem. The sysctl is actually the output of a function, > > not a simple int or the like. > > Yeah, but look at the function (i386 as an example): > > static int > sysctl_hw_physmem(SYSCTL_HANDLER_ARGS) > { > int error = sysctl_handle_int(oidp, 0, ctob(physmem), req); > return (error); > } > > So the physmem value is just in pages; even though the different > platforms use ctob() or _ptob(), the macros all expand to > ((x) << PAGE_SHIFT). True, but I still don't want to spread MD-ish code across the kernel. So, I whipped up the patch I proposed. This introduces "maxmembytes", which contains the size of ram in bytes. I think I've updated all architectures properly, but I've only tested on i386. Could someone working on alpha/ia64/sparc check to make sure that it doesn't break compilation? Unless there are objections, I'll probably commit it in a few days. Thanks, Mike "Silby" Silbersack --0-1762315182-1002043754=:16126 Content-Type: TEXT/PLAIN; charset=US-ASCII; name="maxmembytes.patch" Content-Transfer-Encoding: BASE64 Content-ID: <20011002122914.X16126@achilles.silby.com> Content-Description: Content-Disposition: attachment; filename="maxmembytes.patch" ZGlmZiAtdSAtciBzeXMvYWxwaGEvYWxwaGEvbWFjaGRlcC5jIHN5cy5uZXcv YWxwaGEvYWxwaGEvbWFjaGRlcC5jDQotLS0gc3lzL2FscGhhL2FscGhhL21h Y2hkZXAuYwlXZWQgU2VwIDEyIDA4OjM2OjUzIDIwMDENCisrKyBzeXMubmV3 L2FscGhhL2FscGhhL21hY2hkZXAuYwlUdWUgT2N0ICAyIDEyOjI0OjE5IDIw MDENCkBAIC0xODksNiArMTg5LDcgQEANCiBzdHJ1Y3QgbXNnYnVmICptc2di dWZwPTA7DQogDQogaW50IE1heG1lbSA9IDA7DQordV9pbnQ2NF90IG1heG1l bWJ5dGVzID0gMDsNCiBsb25nIGR1bXBsbzsNCiANCiBpbnQJdG90YWxwaHlz bWVtOwkJLyogdG90YWwgYW1vdW50IG9mIHBoeXNpY2FsIG1lbW9yeSBpbiBz eXN0ZW0gKi8NCkBAIC04NDYsNiArODQ3LDggQEANCiAJCWVsc2UNCiAJCQlN YXhtZW0gPSBhbHBoYV9idG9wKEFsbG93TWVtKTsNCiAJfQ0KKw0KKwltYXht ZW1ieXRlcyA9IGFscGhhX3B0b2IoTWF4bWVtKTsNCiANCiAJd2hpbGUgKHBo eXNtZW0gPiBNYXhtZW0pIHsNCiAJCWludCBpID0gcGh5c19hdmFpbF9jbnQg LSAyOw0KZGlmZiAtdSAtciBzeXMvaTM4Ni9pMzg2L21hY2hkZXAuYyBzeXMu bmV3L2kzODYvaTM4Ni9tYWNoZGVwLmMNCi0tLSBzeXMvaTM4Ni9pMzg2L21h Y2hkZXAuYwlXZWQgU2VwIDEyIDA4OjM3OjI5IDIwMDENCisrKyBzeXMubmV3 L2kzODYvaTM4Ni9tYWNoZGVwLmMJVHVlIE9jdCAgMiAxMjoyNDozOSAyMDAx DQpAQCAtMTU2LDYgKzE1Niw3IEBADQogU1lTQ1RMX0lOVChfbWFjaGRlcCwg T0lEX0FVVE8sIGlzcGM5OCwgQ1RMRkxBR19SRCwgJmlzcGM5OCwgMCwgIiIp Ow0KIA0KIGludCBwaHlzbWVtID0gMDsNCit1X2ludDY0X3QgbWF4bWVtYnl0 ZXMgPSAwOw0KIGludCBjb2xkID0gMTsNCiANCiAjaWZkZWYgQ09NUEFUXzQz DQpAQCAtMTY1NSw2ICsxNjU2LDggQEANCiAJcGh5c19hdmFpbFtwYV9pbmR4 XSAtPSByb3VuZF9wYWdlKE1TR0JVRl9TSVpFKTsNCiANCiAJYXZhaWxfZW5k ID0gcGh5c19hdmFpbFtwYV9pbmR4XTsNCisNCisJbWF4bWVtYnl0ZXMgPSBw dG9hKE1heG1lbSk7DQogfQ0KIA0KIHZvaWQNCmRpZmYgLXUgLXIgc3lzL2lh NjQvaWE2NC9tYWNoZGVwLmMgc3lzLm5ldy9pYTY0L2lhNjQvbWFjaGRlcC5j DQotLS0gc3lzL2lhNjQvaWE2NC9tYWNoZGVwLmMJU2F0IFNlcCAyOSAxMTo0 MzozNiAyMDAxDQorKysgc3lzLm5ldy9pYTY0L2lhNjQvbWFjaGRlcC5jCVR1 ZSBPY3QgIDIgMTI6MDk6MjAgMjAwMQ0KQEAgLTEyOSw2ICsxMjksNyBAQA0K IHN0cnVjdCBtc2didWYgKm1zZ2J1ZnA9MDsNCiANCiBpbnQgYm9vdHZlcmJv c2UgPSAwLCBNYXhtZW0gPSAwOw0KK3VfaW50NjRfdCBtYXhtZW1ieXRlczsN CiBsb25nIGR1bXBsbzsNCiANCiBpbnQJdG90YWxwaHlzbWVtOwkJLyogdG90 YWwgYW1vdW50IG9mIHBoeXNpY2FsIG1lbW9yeSBpbiBzeXN0ZW0gKi8NCkBA IC03MTYsNiArNzE3LDcgQEANCiAJcGh5c19hdmFpbFtwaHlzX2F2YWlsX2Nu dF0gPSAwOw0KIA0KIAlNYXhtZW0gPSBwaHlzbWVtOw0KKwltYXhtZW1ieXRl cyA9IGlhNjRfcHRvYihNYXhtZW0pOw0KIA0KIAkvKg0KIAkgKiBJbml0aWFs aXplIGVycm9yIG1lc3NhZ2UgYnVmZmVyIChhdCBlbmQgb2YgY29yZSkuDQpk aWZmIC11IC1yIHN5cy9wb3dlcnBjL3Bvd2VycGMvbWFjaGRlcC5jIHN5cy5u ZXcvcG93ZXJwYy9wb3dlcnBjL21hY2hkZXAuYw0KLS0tIHN5cy9wb3dlcnBj L3Bvd2VycGMvbWFjaGRlcC5jCU1vbiBTZXAgMjQgMDI6NTg6NDkgMjAwMQ0K KysrIHN5cy5uZXcvcG93ZXJwYy9wb3dlcnBjL21hY2hkZXAuYwlUdWUgT2N0 ICAyIDEyOjA5OjM2IDIwMDENCkBAIC0xMTMsNiArMTEzLDcgQEANCiAjaW5j bHVkZSA8bWFjaGluZS9zaWdmcmFtZS5oPg0KIA0KIGludCBwaHlzbWVtID0g MDsNCit1X2ludDY0X3QgbWF4bWVtYnl0ZXMgPSAwOw0KIGludCBjb2xkID0g MTsNCiANCiBzdHJ1Y3QgbXR4CXNjaGVkX2xvY2s7DQpkaWZmIC11IC1yIHN5 cy9zcGFyYzY0L3NwYXJjNjQvbWFjaGRlcC5jIHN5cy5uZXcvc3BhcmM2NC9z cGFyYzY0L21hY2hkZXAuYw0KLS0tIHN5cy9zcGFyYzY0L3NwYXJjNjQvbWFj aGRlcC5jCVN1biBTZXAgMzAgMTg6NDg6MzcgMjAwMQ0KKysrIHN5cy5uZXcv c3BhcmM2NC9zcGFyYzY0L21hY2hkZXAuYwlUdWUgT2N0ICAyIDEyOjA3OjA3 IDIwMDENCkBAIC05OCw2ICs5OCw3IEBADQogaW50IGNvbGQgPSAxOw0KIGxv bmcgZHVtcGxvOw0KIGludCBNYXhtZW07DQordV9pbnQ2NF90IG1heG1lbWJ5 dGVzOw0KIA0KIHVfbG9uZyBkZWJ1Z19tYXNrOw0KIA0KZGlmZiAtdSAtciBz eXMvc3lzL3N5c3RtLmggc3lzLm5ldy9zeXMvc3lzdG0uaA0KLS0tIHN5cy9z eXMvc3lzdG0uaAlUaHUgU2VwIDIwIDIxOjQ1OjMxIDIwMDENCisrKyBzeXMu bmV3L3N5cy9zeXN0bS5oCVR1ZSBPY3QgIDIgMTI6MDk6MjYgMjAwMQ0KQEAg LTYwLDYgKzYwLDcgQEANCiBleHRlcm4gc3RydWN0IGN2IHNlbHdhaXQ7CS8q IHNlbGVjdCBjb25kaXRpb25hbCB2YXJpYWJsZSAqLw0KIA0KIGV4dGVybiBp bnQgcGh5c21lbTsJCS8qIHBoeXNpY2FsIG1lbW9yeSAqLw0KK2V4dGVybiB1 X2ludDY0X3QgbWF4bWVtYnl0ZXM7CS8qIG1heGltdW0gbWVtb3J5IGluIGJ5 dGVzICovDQogDQogZXh0ZXJuIGRldl90IGR1bXBkZXY7CQkvKiBkdW1wIGRl dmljZSAqLw0KIGV4dGVybiBsb25nIGR1bXBsbzsJCS8qIG9mZnNldCBpbnRv IGR1bXBkZXYgKi8NCg== --0-1762315182-1002043754=:16126-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 10:38:43 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mail.rpi.edu (mail.rpi.edu [128.113.22.40]) by hub.freebsd.org (Postfix) with ESMTP id 578DC37B407; Tue, 2 Oct 2001 10:38:36 -0700 (PDT) Received: from [128.113.24.47] (gilead.acs.rpi.edu [128.113.24.47]) by mail.rpi.edu (8.11.3/8.11.3) with ESMTP id f92HcNk111260; Tue, 2 Oct 2001 13:38:23 -0400 Mime-Version: 1.0 X-Sender: drosih@mail.rpi.edu Message-Id: In-Reply-To: References: Date: Tue, 2 Oct 2001 13:38:20 -0400 To: John Baldwin , Peter Wemm From: Garance A Drosihn Subject: Re: Style Wars Cc: arch@FreeBSD.ORG, Warner Losh , Julian Elischer Content-Type: text/plain; charset="us-ascii" ; format="flowed" Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG At 4:54 PM -0700 9/28/01, John Baldwin wrote: >I'm think 1b) is the one most people have favored so far and it is >rather close to our existing style, so it's not that big of a change. >Does anyone object to 1b)? It basically results in the following >changes: use 2 tab spaces instead of 1 for type names, put the entire >type name before the tab(s), and if the type is too long, just use >a space. > >> 1b) >> >> struct foo { >> int f_type; >> struct mtx f_lock; >> const char *f_name; >> volatile int f_int; > > u_int64_t f_64; >> const volatile char f_cv; >> TAILQ_ENTRY(foo) f_link; > > }; This is the guideline that I prefer the most. It probably should include an example of a function-pointer. u_int64_t f_64; void (*fun_ptr)(int, char *[]); (which, I assume, would show that you would be lining up the name of the function pointer with the other variable names, so the argument list continues on to the right). -- Garance Alistair Drosehn = gad@eclipse.acs.rpi.edu Senior Systems Programmer or gad@freebsd.org Rensselaer Polytechnic Institute or drosih@rpi.edu To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 11:18:31 2001 Delivered-To: freebsd-arch@freebsd.org Received: from dragon.nuxi.com (trang.nuxi.com [66.92.13.169]) by hub.freebsd.org (Postfix) with ESMTP id 4B45737B407 for ; Tue, 2 Oct 2001 11:18:29 -0700 (PDT) Received: (from obrien@localhost) by dragon.nuxi.com (8.11.6/8.11.1) id f92IIRZ94727; Tue, 2 Oct 2001 11:18:27 -0700 (PDT) (envelope-from obrien) Date: Tue, 2 Oct 2001 11:18:27 -0700 From: "David O'Brien" To: Julian Elischer Cc: arch@freebsd.org Subject: Re: KSE next steps... Message-ID: <20011002111827.B94118@dragon.nuxi.com> Reply-To: obrien@freebsd.org References: <20011001144235.B97970@dragon.nuxi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: ; from julian@elischer.org on Mon, Oct 01, 2001 at 05:14:00PM -0700 X-Operating-System: FreeBSD 5.0-CURRENT Organization: The NUXI BSD group X-Pgp-Rsa-Fingerprint: B7 4D 3E E9 11 39 5F A3 90 76 5D 69 58 D9 98 7A X-Pgp-Rsa-Keyid: 1024/34F9F9D5 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Mon, Oct 01, 2001 at 05:14:00PM -0700, Julian Elischer wrote: > > > > Does this replace kse_init() in the paper? > > basically thre are a number of things that need to be done to get > a process off and running in KSE mode. I'm leaning towards packaging > them in a slightly different set of syscalls to thiose in the paper, > even though all the same steps need to be done by the time that > we're finished.. How about updating the paper first, so we'll all know where this is heading? This is one of the few subsystems we (FreeBSD) has done that actually has a good paper associated with it. It would be quite a shame to make it worthless. -- -- David (obrien@FreeBSD.org) To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 12: 2: 6 2001 Delivered-To: freebsd-arch@freebsd.org Received: from InterJet.elischer.org (c421509-a.pinol1.sfba.home.com [24.7.86.9]) by hub.freebsd.org (Postfix) with ESMTP id 09BEF37B406 for ; Tue, 2 Oct 2001 12:02:04 -0700 (PDT) Received: from localhost (localhost.elischer.org [127.0.0.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id MAA92558; Tue, 2 Oct 2001 12:45:23 -0700 (PDT) Date: Tue, 2 Oct 2001 12:45:23 -0700 (PDT) From: Julian Elischer To: Mark Murray Cc: arch@freebsd.org Subject: Re: [Patch] style.9 nit In-Reply-To: <200110011648.f91Gmgt25779@grimreaper.grondar.za> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG names are suggested for kernel code... On Mon, 1 Oct 2001, Mark Murray wrote: > Hi > > Any objections to this patch? > > It removes argument names from an example function prototype > to match a previously mentioned guideline about such names. > > eg: > > int foo(int bar); > > becomes > > int foo(int); > > This helps quite a bit in reducing namespace pollution. > > M > > Index: style.9 > =================================================================== > RCS file: /home/ncvs/src/share/man/man9/style.9,v > retrieving revision 1.66 > diff -u -d -r1.66 style.9 > --- style.9 1 Oct 2001 16:13:59 -0000 1.66 > +++ style.9 1 Oct 2001 16:39:38 -0000 > @@ -252,8 +252,7 @@ > Prototypes may have an extra space after a tab to enable function names > to line up: > .Bd -literal > -static char *function(int _arg, const char *_arg2, struct foo *_arg3, > - struct bar *_arg4); > +static char *function(int, const char *, struct foo *, struct bar *); > static void usage(void); > > /* > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-arch" in the body of the message > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 12:39: 0 2001 Delivered-To: freebsd-arch@freebsd.org Received: from InterJet.elischer.org (c421509-a.pinol1.sfba.home.com [24.7.86.9]) by hub.freebsd.org (Postfix) with ESMTP id 9924F37B401; Tue, 2 Oct 2001 12:38:57 -0700 (PDT) Received: from localhost (localhost.elischer.org [127.0.0.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id NAA92751; Tue, 2 Oct 2001 13:25:38 -0700 (PDT) Date: Tue, 2 Oct 2001 13:25:36 -0700 (PDT) From: Julian Elischer To: "David O'Brien" Cc: arch@freebsd.org Subject: Re: KSE next steps... In-Reply-To: <20011002111827.B94118@dragon.nuxi.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Tue, 2 Oct 2001, David O'Brien wrote: > On Mon, Oct 01, 2001 at 05:14:00PM -0700, Julian Elischer wrote: > > > > > > Does this replace kse_init() in the paper? > > > > basically thre are a number of things that need to be done to get > > a process off and running in KSE mode. I'm leaning towards packaging > > them in a slightly different set of syscalls to thiose in the paper, > > even though all the same steps need to be done by the time that > > we're finished.. > > How about updating the paper first, so we'll all know where this is > heading? This is one of the few subsystems we (FreeBSD) has done that > actually has a good paper associated with it. It would be quite a shame > to make it worthless. I'll update the paper when we know what the api is.. > > -- > -- David (obrien@FreeBSD.org) > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 16:15:57 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mass.dis.org (mass.dis.org [216.240.45.41]) by hub.freebsd.org (Postfix) with ESMTP id 3ADD737B407 for ; Tue, 2 Oct 2001 16:15:54 -0700 (PDT) Received: from mass.dis.org (localhost [127.0.0.1]) by mass.dis.org (8.11.6/8.11.3) with ESMTP id f92NPSt02665; Tue, 2 Oct 2001 16:25:35 -0700 (PDT) (envelope-from msmith@mass.dis.org) Message-Id: <200110022325.f92NPSt02665@mass.dis.org> X-Mailer: exmh version 2.1.1 10/15/1999 To: Mark Murray Cc: arch@freebsd.org Subject: Re: [Patch] style.9 nit In-reply-to: Your message of "Mon, 01 Oct 2001 17:48:42 BST." <200110011648.f91Gmgt25779@grimreaper.grondar.za> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Tue, 02 Oct 2001 16:25:28 -0700 From: Mike Smith Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG > Hi > > Any objections to this patch? > > It removes argument names from an example function prototype > to match a previously mentioned guideline about such names. This is a bit of a contentious issue. Personally, I find it useful to be able to tell what the argument(s) to a function are from the prototype. All this really saves us from would be, in your example, someone redefining 'bar' with a preprocessor macro; something they typically shouldn't be doing anyway (based on other guidelines). For documented functions that are part of a public interface and consumed by third-party code, I think this is a good idea. For other code, I'm less sure about it. > eg: > > int foo(int bar); > > becomes > > int foo(int); > > This helps quite a bit in reducing namespace pollution. > > M > > Index: style.9 > =================================================================== > RCS file: /home/ncvs/src/share/man/man9/style.9,v > retrieving revision 1.66 > diff -u -d -r1.66 style.9 > --- style.9 1 Oct 2001 16:13:59 -0000 1.66 > +++ style.9 1 Oct 2001 16:39:38 -0000 > @@ -252,8 +252,7 @@ > Prototypes may have an extra space after a tab to enable function names > to line up: > .Bd -literal > -static char *function(int _arg, const char *_arg2, struct foo *_arg3, > - struct bar *_arg4); > +static char *function(int, const char *, struct foo *, struct bar *); > static void usage(void); > > /* > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-arch" in the body of the message -- ... every activity meets with opposition, everyone who acts has his rivals and unfortunately opponents also. But not because people want to be opponents, rather because the tasks and relationships force people to take different points of view. [Dr. Fritz Todt] V I C T O R Y N O T V E N G E A N C E To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Tue Oct 2 23:42:23 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mx.wgate.com (mail.wgate.com [38.219.83.4]) by hub.freebsd.org (Postfix) with SMTP id C576A37B403 for ; Tue, 2 Oct 2001 23:42:17 -0700 (PDT) To: Warner Losh Cc: Julian Elischer , Paul Richards , John Baldwin , arch@FreeBSD.ORG Received: From MAIL.TVOL.NET (10.1.1.4[10.1.1.4 port:4760]) by mx.wgate.comMail essentials (server 2.429) with SMTP id: <29864@mx.wgate.com>transfer for ; Wed, 3 Oct 2001 2:40:27 AM -0400 ;transfer smtpmailfrom X-MESINK_Inbound: 0 X-MESINK_MailForType: SMTP X-MESINK_SenderType: SMTP X-MESINK_Sender: rjesup@wgate.com X-MESINK_MailFor: arch@FreeBSD.ORG Received: from jesup.eng.tvol.net ([10.32.2.26]) by mail.tvol.net with SMTP (Microsoft Exchange Internet Mail Service Version 5.5.2653.13)id 4BWN0M60; Wed, 3 Oct 2001 02:40:34 -0400 Reply-To: Randell Jesup Subject: Re: Style Wars References: <6bdeb355057b0307d1@[192.168.1.4]> From: Randell Jesup Date: 03 Oct 2001 02:46:56 -0400 In-Reply-To: Warner Losh's message of "Fri, 28 Sep 2001 17:31:45 -0600" User-Agent: Gnus/5.0807 (Gnus v5.8.7) Emacs/20.7 Content-Type: text/plain; charset=us-ascii x-receiver: arch@FreeBSD.ORG x-sender: rjesup@wgate.com Content-Transfer-Encoding: Quoted-Printable MIME-Version: 1.0 Message-ID: <7fffe858038bd607d1@[192.168.1.4]> Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Warner Losh writes: >: > I think stating intent, rather than method will reduce a lot of the >: > quibbling over style issues because people will understand what's bein= g >: > aimed for and won't bridle so much over dogmatic rules. > >The difficult part in doing intent based rules is that in the past we >was many style commits back and forth as different people interpreted >the rules differently. The more rigid the rules, the more anybody >will know if things are in compliance. I consider these to be different issues. If you have a purposely vague rule ("line things up and order them so as to be readable and improve= understandibility of the code" or some such), then it's really hard to be "out of compliance". Someone may disagree over what's more readable, but it's hard to argue unless it's really screwey. You could state that when modifying code, you should attempt to follow the existing style within the file. However, I think both a _rigid_ style and/or lots of carping over style (style commits, major style rework of submissions, etc) merely serves= to slow down development and reduce interest in submitting patches, with little if any gain. I can't say that any of the styles here being discussed would improve matters much if at all over letting people choose a= style that promotes understanding that particular bit of code. I know for certain that after the experience of getting whacked over style issues on the one patch I submitted I became much less interested in submitting patches in the future. Also, even the excessively= rigid style dictums for code (as opposed to structs) caused several people to disagree over whether they were followed or not. None of the discussion= was over whether my code was readable; I'm quite certain that a number of the changes requested for style(9) issues _reduced_ the clarity of the code= to anyone (not just to me). State intent, and give examples and preferences, but don't spend immense numbers of hours worrying about style(9) like we do now. Stop worrying about rearranging the deck chairs. There are bigger fish to fry. IMHO. (Yes, I know my comments here are futile, but I have to try.) -- Randell Jesup, Worldgate Communications, ex-Scala, ex-Amiga OS team rjesup@wgate.com "They that can give up essential liberty to obtain a little temporary safet= y deserve neither liberty nor safety." -Benjamin Franklin, Historical Review of Pennsylvania, 1759. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Wed Oct 3 1:24:22 2001 Delivered-To: freebsd-arch@freebsd.org Received: from arb.arb.za.net (arb.arb.za.net [196.7.148.4]) by hub.freebsd.org (Postfix) with ESMTP id 6B23E37B401; Wed, 3 Oct 2001 01:24:14 -0700 (PDT) Received: (from uucp@localhost) by arb.arb.za.net (8.11.3/8.11.3) with UUCP id f938OBC84859; Wed, 3 Oct 2001 10:24:11 +0200 (SAST) (envelope-from mark@grondar.za) Received: from grondar.za (localhost [127.0.0.1]) by grimreaper.grondar.za (8.11.6/8.11.6) with ESMTP id f938KvA47987; Wed, 3 Oct 2001 09:20:58 +0100 (BST) (envelope-from mark@grondar.za) Message-Id: <200110030820.f938KvA47987@grimreaper.grondar.za> To: Mike Smith Cc: arch@freebsd.org From: mark@freebsd.org Subject: Re: [Patch] style.9 nit References: <200110022325.f92NPSt02665@mass.dis.org> In-Reply-To: <200110022325.f92NPSt02665@mass.dis.org> ; from Mike Smith "Tue, 02 Oct 2001 16:25:28 PDT." Date: Wed, 03 Oct 2001 09:20:56 +0100 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG > > Hi > > > > Any objections to this patch? > > > > It removes argument names from an example function prototype > > to match a previously mentioned guideline about such names. > > This is a bit of a contentious issue. Personally, I find it useful to be > able to tell what the argument(s) to a function are from the prototype. Ok - I like the protected name approach (int foo (int _bar) instead of int foo(int bar) ). Does that work for you? The namespace pollution I refer to is WARNS=2 complaints about variable "bar" overriding a global/previous definition. > All this really saves us from would be, in your example, someone > redefining 'bar' with a preprocessor macro; something they typically > shouldn't be doing anyway (based on other guidelines). > > For documented functions that are part of a public interface and consumed > by third-party code, I think this is a good idea. For other code, I'm > less sure about it. I'd like to protect the argument names in header files that are causing WARNS=2 breakage. M > > eg: > > > > int foo(int bar); > > > > becomes > > > > int foo(int); > > > > This helps quite a bit in reducing namespace pollution. > > > > M > > > > Index: style.9 > > =================================================================== > > RCS file: /home/ncvs/src/share/man/man9/style.9,v > > retrieving revision 1.66 > > diff -u -d -r1.66 style.9 > > --- style.9 1 Oct 2001 16:13:59 -0000 1.66 > > +++ style.9 1 Oct 2001 16:39:38 -0000 > > @@ -252,8 +252,7 @@ > > Prototypes may have an extra space after a tab to enable function names > > to line up: > > .Bd -literal > > -static char *function(int _arg, const char *_arg2, struct foo *_arg3, > > - struct bar *_arg4); > > +static char *function(int, const char *, struct foo *, struct bar *); > > static void usage(void); > > > > /* > > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > > with "unsubscribe freebsd-arch" in the body of the message > > -- > ... every activity meets with opposition, everyone who acts has his > rivals and unfortunately opponents also. But not because people want > to be opponents, rather because the tasks and relationships force > people to take different points of view. [Dr. Fritz Todt] > V I C T O R Y N O T V E N G E A N C E > > -- o Mark Murray \_ FreeBSD Services Limited O.\_ Warning: this .sig is umop ap!sdn To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Wed Oct 3 4:48:52 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 234C637B406 for ; Wed, 3 Oct 2001 04:48:49 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id VAA27227; Wed, 3 Oct 2001 21:48:27 +1000 Date: Wed, 3 Oct 2001 21:47:49 +1000 (EST) From: Bruce Evans X-X-Sender: To: Mike Silbersack Cc: Dan Nelson , Subject: Re: Reading physical memory in a cross-platform way In-Reply-To: <20011002122651.P16126-200000@achilles.silby.com> Message-ID: <20011003212829.R97858-100000@delplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Tue, 2 Oct 2001, Mike Silbersack wrote: > On Mon, 1 Oct 2001, Dan Nelson wrote: > > > > physmem != hw.physmem. The sysctl is actually the output of a function, > > > not a simple int or the like. > > > > Yeah, but look at the function (i386 as an example): > > > > static int > > sysctl_hw_physmem(SYSCTL_HANDLER_ARGS) > > { > > int error = sysctl_handle_int(oidp, 0, ctob(physmem), req); > > return (error); > > } > > > > So the physmem value is just in pages; even though the different > > platforms use ctob() or _ptob(), the macros all expand to > > ((x) << PAGE_SHIFT). Using ptob() (pages to bytes) would be correct, except ptob() doesn't exist. There are "only" _ptob(), ptoa() (pages to address) and the anachronism ctob() (clicks to bytes), not to mention explicit shifts and multiplications, to do this conversion. > True, but I still don't want to spread MD-ish code across the kernel. So, > I whipped up the patch I proposed. This introduces "maxmembytes", which > contains the size of ram in bytes. I think I've updated all architectures > properly, but I've only tested on i386. Could someone working on > alpha/ia64/sparc check to make sure that it doesn't break compilation? > > Unless there are objections, I'll probably commit it in a few days. No thanks. This spreads MI-ish code across all arches. Note that physmem is already used in MI code, in vfs_bio_alloc() and vm_ksubmap_init(). vfs_bio_alloc() is not actually MI -- machine- dependencies in this code include the magic number 16384 (64MB in i386 pages). vm_ksubmap_init() uses btoc() (bytes to clicks) to do the reverse conversion to the one you want. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Wed Oct 3 6:11:37 2001 Delivered-To: freebsd-arch@freebsd.org Received: from silby.com (cb34181-a.mdsn1.wi.home.com [24.14.173.39]) by hub.freebsd.org (Postfix) with ESMTP id 743C637B407 for ; Wed, 3 Oct 2001 06:11:34 -0700 (PDT) Received: (qmail 19353 invoked by uid 1000); 3 Oct 2001 13:11:33 -0000 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 3 Oct 2001 13:11:33 -0000 Date: Wed, 3 Oct 2001 08:11:33 -0500 (CDT) From: Mike Silbersack To: Bruce Evans Cc: Dan Nelson , Subject: Re: Reading physical memory in a cross-platform way In-Reply-To: <20011003212829.R97858-100000@delplex.bde.org> Message-ID: <20011003080703.F19313-100000@achilles.silby.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Wed, 3 Oct 2001, Bruce Evans wrote: > > True, but I still don't want to spread MD-ish code across the kernel. So, > > I whipped up the patch I proposed. This introduces "maxmembytes", which > > contains the size of ram in bytes. I think I've updated all architectures > > properly, but I've only tested on i386. Could someone working on > > alpha/ia64/sparc check to make sure that it doesn't break compilation? > > > > Unless there are objections, I'll probably commit it in a few days. > > No thanks. This spreads MI-ish code across all arches. > > Note that physmem is already used in MI code, in vfs_bio_alloc() and > vm_ksubmap_init(). vfs_bio_alloc() is not actually MI -- machine- > dependencies in this code include the magic number 16384 (64MB in > i386 pages). vm_ksubmap_init() uses btoc() (bytes to clicks) to > do the reverse conversion to the one you want. > > Bruce Hrm. Yes, physmem is used in those functions, but it requires quite a bit of conversion. I'm going to be calculating these values in about 4 seperate files, and I don't want to clutter each calculation with the conversion. Doesn't it make more sense from a portability perspective to keep the calculations entirely within machdep.c so that just a simple variable can be exported? If not, where should I centralize the calculation? There's one more small issue: at least on i386, physmem reports a value a few MB below the actual size, while maxmem reports the correct value. So, using physmem is less than desireable for me in this case. Mike "Silby" Silbersack To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Wed Oct 3 11: 6:11 2001 Delivered-To: freebsd-arch@freebsd.org Received: from avocet.mail.pas.earthlink.net (avocet.mail.pas.earthlink.net [207.217.121.50]) by hub.freebsd.org (Postfix) with ESMTP id A55E837B403 for ; Wed, 3 Oct 2001 11:06:05 -0700 (PDT) Received: from dialup-209.245.142.238.dial1.sanjose1.level3.net ([209.245.142.238] helo=mindspring.com) by avocet.mail.pas.earthlink.net with esmtp (Exim 3.32 #2) id 15oqPD-0002s2-00; Wed, 03 Oct 2001 11:06:03 -0700 Message-ID: <3BBB53BB.84647DC1@mindspring.com> Date: Wed, 03 Oct 2001 11:06:51 -0700 From: Terry Lambert Reply-To: tlambert2@mindspring.com X-Mailer: Mozilla 4.7 [en]C-CCK-MCD {Sony} (Win98; U) X-Accept-Language: en MIME-Version: 1.0 To: Mike Silbersack Cc: freebsd-arch@freebsd.org Subject: Re: Reading physical memory in a cross-platform way References: <20011001214234.W98394-100000@achilles.silby.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Mike Silbersack wrote: > 1. Is there a variable / function which contains the size of memory > across all platforms that I am missing? sysctl -A | grep -i mem > 2. If not, is there a problem if I add a u_int64_t containing the size of > physical memory in bytes in machdep.c for each architecture? If this number isn't what you are looking for, I have a patch that adds a sysctl for the number of pages of physical memory, the same as is reported by the kernel at boot time, which I could send you. Let me know at my clickarray.com address (which is "terry"). -- Terry To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Wed Oct 3 11:20: 2 2001 Delivered-To: freebsd-arch@freebsd.org Received: from silby.com (cb34181-a.mdsn1.wi.home.com [24.14.173.39]) by hub.freebsd.org (Postfix) with ESMTP id B9DDC37B403 for ; Wed, 3 Oct 2001 11:19:57 -0700 (PDT) Received: (qmail 345 invoked by uid 1000); 3 Oct 2001 18:19:57 -0000 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 3 Oct 2001 18:19:57 -0000 Date: Wed, 3 Oct 2001 13:19:57 -0500 (CDT) From: Mike Silbersack To: Terry Lambert Cc: Subject: Re: Reading physical memory in a cross-platform way In-Reply-To: <3BBB53BB.84647DC1@mindspring.com> Message-ID: <20011003131829.D256-100000@achilles.silby.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Wed, 3 Oct 2001, Terry Lambert wrote: > Mike Silbersack wrote: > > 1. Is there a variable / function which contains the size of memory > > across all platforms that I am missing? > > sysctl -A | grep -i mem Er, I guess I should explain more. This is code inside the kernel in various places. I'm just trying to find a quick way to have the number of bytes of physical memory available on all platforms without any conversions necessary. Mike "Silby" Silbersack To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Wed Oct 3 12:38:30 2001 Delivered-To: freebsd-arch@freebsd.org Received: from robin.mail.pas.earthlink.net (robin.mail.pas.earthlink.net [207.217.120.65]) by hub.freebsd.org (Postfix) with ESMTP id 1728537B40B for ; Wed, 3 Oct 2001 12:38:28 -0700 (PDT) Received: from mindspring.com (dialup-209.245.142.238.Dial1.SanJose1.Level3.net [209.245.142.238]) by robin.mail.pas.earthlink.net (8.11.5/8.9.3) with ESMTP id f93JcPP21685; Wed, 3 Oct 2001 12:38:25 -0700 (PDT) Message-ID: <3BBB6962.9894CD87@mindspring.com> Date: Wed, 03 Oct 2001 12:39:14 -0700 From: Terry Lambert Reply-To: tlambert2@mindspring.com X-Mailer: Mozilla 4.7 [en]C-CCK-MCD {Sony} (Win98; U) X-Accept-Language: en MIME-Version: 1.0 To: Mike Silbersack Cc: freebsd-arch@freebsd.org Subject: Re: Reading physical memory in a cross-platform way References: <20011003131829.D256-100000@achilles.silby.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Mike Silbersack wrote: > > On Wed, 3 Oct 2001, Terry Lambert wrote: > > > Mike Silbersack wrote: > > > 1. Is there a variable / function which contains the size of memory > > > across all platforms that I am missing? > > > > sysctl -A | grep -i mem > > Er, I guess I should explain more. This is code inside the kernel in > various places. I'm just trying to find a quick way to have the number of > bytes of physical memory available on all platforms without any > conversions necessary. Then you want my patch; I will send it to you from work. -- Terry To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Wed Oct 3 18: 5:55 2001 Delivered-To: freebsd-arch@freebsd.org Received: from green.bikeshed.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id 84C1537B405 for ; Wed, 3 Oct 2001 18:05:39 -0700 (PDT) Received: from localhost (green@localhost) by green.bikeshed.org (8.11.4/8.11.1) with ESMTP id f9415av15297 for ; Wed, 3 Oct 2001 21:05:38 -0400 (EDT) (envelope-from green@green.bikeshed.org) Message-Id: <200110040105.f9415av15297@green.bikeshed.org> X-Mailer: exmh version 2.5 07/13/2001 with nmh-1.0.4 To: arch@FreeBSD.org Subject: sx-ifying src/sys/vm From: Brian Fundakowski Feldman Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Wed, 03 Oct 2001 21:05:36 -0400 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG I've taken a shot at changing the vm_map locking from using lockmgr() locks (EAAARGHH...) to sx locks. Already I can tell that it 1. seems to work and 2. seems to not give me bogus lock ordering messages like lockmgr() used to. I'd appreciate if people took a look at it and told me of any concerns, whether it also looks fine to more than just myself, etc. I added a few more vm_map locking operations than were there already for the purpose of removing the hard-coded lockmgr() ops that were in a few places previously. Instead of having a sx_locked() operation I used sx_try_xlock(), and instead of recursion I saved the state of the one case where a lock was attempted to be "recursed" upon, and removed the actual recursion on the lock. I also used EWOULDBLOCK instead of EBUSY as a return value for operations that would block on attempting to acquire a lock, which I think makes more sense, but in any case is not used currently in any of the code. I could go either way on that :) Here are all the changes for review: Index: vm_fault.c =================================================================== RCS file: /home/freebsd/ncvs/src/sys/vm/vm_fault.c,v retrieving revision 1.125 diff -u -r1.125 vm_fault.c --- vm_fault.c 2001/09/12 08:38:11 1.125 +++ vm_fault.c 2001/10/04 00:41:36 @@ -114,6 +114,7 @@ vm_map_entry_t entry; int lookup_still_valid; struct vnode *vp; + int lockheld; }; static __inline void @@ -127,7 +128,7 @@ static __inline void unlock_map(struct faultstate *fs) { - if (fs->lookup_still_valid) { + if (fs->lookup_still_valid && !fs->lockheld) { vm_map_lookup_done(fs->map, fs->entry); fs->lookup_still_valid = FALSE; } @@ -213,12 +214,16 @@ int hardfault; int faultcount; struct faultstate fs; + boolean_t user_wire; GIANT_REQUIRED; cnt.v_vm_faults++; hardfault = 0; + user_wire = (fault_flags & VM_FAULT_WIRE_MASK) == VM_FAULT_USER_WIRE; + fs.lockheld = user_wire; + RetryFault:; /* @@ -226,13 +231,11 @@ * search. */ fs.map = map; - if ((result = vm_map_lookup(&fs.map, vaddr, + if ((result = vm_map_lookup2(&fs.map, vaddr, fault_type, &fs.entry, &fs.first_object, - &fs.first_pindex, &prot, &wired)) != KERN_SUCCESS) { - if ((result != KERN_PROTECTION_FAILURE) || - ((fault_flags & VM_FAULT_WIRE_MASK) != VM_FAULT_USER_WIRE)) { + &fs.first_pindex, &prot, &wired, user_wire)) != KERN_SUCCESS) { + if (result != KERN_PROTECTION_FAILURE || !user_wire) return result; - } /* * If we are user-wiring a r/w segment, and it is COW, then @@ -241,9 +244,10 @@ * to COW .text. We simply keep .text from ever being COW'ed * and take the heat that one cannot debug wired .text sections. */ - result = vm_map_lookup(&fs.map, vaddr, + result = vm_map_lookup2(&fs.map, vaddr, VM_PROT_READ|VM_PROT_WRITE|VM_PROT_OVERRIDE_WRITE, - &fs.entry, &fs.first_object, &fs.first_pindex, &prot, &wired); + &fs.entry, &fs.first_object, &fs.first_pindex, &prot, + &wired, user_wire); if (result != KERN_SUCCESS) { return result; } @@ -666,7 +670,7 @@ * grab the lock if we need to */ (fs.lookup_still_valid || - lockmgr(&fs.map->lock, LK_EXCLUSIVE|LK_NOWAIT, (void *)0, curthread) == 0) + vm_map_lock_upgrade(fs.map) == 0) ) { fs.lookup_still_valid = 1; Index: vm_glue.c =================================================================== RCS file: /home/freebsd/ncvs/src/sys/vm/vm_glue.c,v retrieving revision 1.119 diff -u -r1.119 vm_glue.c --- vm_glue.c 2001/09/12 08:38:11 1.119 +++ vm_glue.c 2001/10/04 00:41:37 @@ -577,9 +577,7 @@ * data structures there is a * possible deadlock. */ - if (lockmgr(&vm->vm_map.lock, - LK_EXCLUSIVE | LK_NOWAIT, - NULL, curthread)) { + if (vm_map_trylock(&vm->vm_map)) { vmspace_free(vm); PROC_UNLOCK(p); goto nextproc; Index: vm_map.c =================================================================== RCS file: /home/freebsd/ncvs/src/sys/vm/vm_map.c,v retrieving revision 1.207 diff -u -r1.207 vm_map.c --- vm_map.c 2001/09/12 08:38:11 1.207 +++ vm_map.c 2001/10/04 00:41:44 @@ -266,70 +266,59 @@ vm_map_lock(vm_map_t map) { vm_map_printf("locking map LK_EXCLUSIVE: %p\n", map); - if (lockmgr(&map->lock, LK_EXCLUSIVE, NULL, curthread) != 0) - panic("vm_map_lock: failed to get lock"); + sx_xlock(&map->lock); map->timestamp++; } +int +vm_map_trylock(vm_map_t map) +{ + vm_map_printf("trying to lock map LK_EXCLUSIVE: %p\n", map); + if (sx_try_xlock(&map->lock)) { + map->timestamp++; + return (0); + } + return (EWOULDBLOCK); +} + void vm_map_unlock(vm_map_t map) { vm_map_printf("locking map LK_RELEASE: %p\n", map); - lockmgr(&(map)->lock, LK_RELEASE, NULL, curthread); + sx_xunlock(&map->lock); } void vm_map_lock_read(vm_map_t map) { vm_map_printf("locking map LK_SHARED: %p\n", map); - lockmgr(&(map)->lock, LK_SHARED, NULL, curthread); + sx_slock(&map->lock); } void vm_map_unlock_read(vm_map_t map) { vm_map_printf("locking map LK_RELEASE: %p\n", map); - lockmgr(&(map)->lock, LK_RELEASE, NULL, curthread); -} - -static __inline__ int -_vm_map_lock_upgrade(vm_map_t map, struct thread *td) { - int error; - - vm_map_printf("locking map LK_EXCLUPGRADE: %p\n", map); - error = lockmgr(&map->lock, LK_EXCLUPGRADE, NULL, td); - if (error == 0) - map->timestamp++; - return error; + sx_sunlock(&map->lock); } int vm_map_lock_upgrade(vm_map_t map) { - return(_vm_map_lock_upgrade(map, curthread)); + vm_map_printf("locking map LK_EXCLUPGRADE: %p\n", map); + if (sx_try_upgrade(&map->lock)) { + map->timestamp++; + return (0); + } + sx_sunlock(&map->lock); + return (EWOULDBLOCK); } void vm_map_lock_downgrade(vm_map_t map) { vm_map_printf("locking map LK_DOWNGRADE: %p\n", map); - lockmgr(&map->lock, LK_DOWNGRADE, NULL, curthread); -} - -void -vm_map_set_recursive(vm_map_t map) -{ - mtx_lock((map)->lock.lk_interlock); - map->lock.lk_flags |= LK_CANRECURSE; - mtx_unlock((map)->lock.lk_interlock); -} - -void -vm_map_clear_recursive(vm_map_t map) -{ - mtx_lock((map)->lock.lk_interlock); - map->lock.lk_flags &= ~LK_CANRECURSE; - mtx_unlock((map)->lock.lk_interlock); + sx_downgrade(&map->lock); } vm_offset_t @@ -403,7 +392,7 @@ map->first_free = &map->header; map->hint = &map->header; map->timestamp = 0; - lockinit(&map->lock, PVM, "thrd_sleep", 0, LK_NOPAUSE); + sx_init(&map->lock, "thrd_sleep"); } void @@ -411,7 +400,7 @@ struct vm_map *map; { GIANT_REQUIRED; - lockdestroy(&map->lock); + sx_destroy(&map->lock); } /* @@ -1490,7 +1479,6 @@ estart = entry->start; /* First we need to allow map modifications */ - vm_map_set_recursive(map); vm_map_lock_downgrade(map); map->timestamp++; @@ -1500,14 +1488,12 @@ entry->wired_count--; entry->eflags &= ~MAP_ENTRY_USER_WIRED; - vm_map_clear_recursive(map); - vm_map_unlock(map); + vm_map_unlock_read(map); (void) vm_map_user_pageable(map, start, entry->start, TRUE); return rv; } - vm_map_clear_recursive(map); if (vm_map_lock_upgrade(map)) { vm_map_lock(map); if (vm_map_lookup_entry(map, estart, &entry) @@ -1748,14 +1734,20 @@ vm_map_lock(map); } if (rv) { - vm_map_unlock(map); + if (vm_map_pmap(map) == kernel_pmap) + vm_map_unlock(map); + else + vm_map_unlock_read(map); (void) vm_map_pageable(map, start, failed, TRUE); return (rv); } vm_map_simplify_entry(map, start_entry); } - vm_map_unlock(map); + if (new_pageable || vm_map_pmap(map) == kernel_pmap) + vm_map_unlock(map); + else + vm_map_unlock_read(map); return (KERN_SUCCESS); } @@ -2678,6 +2670,22 @@ vm_prot_t *out_prot, /* OUT */ boolean_t *wired) /* OUT */ { + + return (vm_map_lookup2(var_map, vaddr, fault_typea, out_entry, + object, pindex, out_prot, wired, 0)); +} + +int +vm_map_lookup2(vm_map_t *var_map, /* IN/OUT */ + vm_offset_t vaddr, + vm_prot_t fault_typea, + vm_map_entry_t *out_entry, /* OUT */ + vm_object_t *object, /* OUT */ + vm_pindex_t *pindex, /* OUT */ + vm_prot_t *out_prot, /* OUT */ + boolean_t *wired, /* OUT */ + boolean_t lockheld) +{ vm_map_entry_t entry; vm_map_t map = *var_map; vm_prot_t prot; @@ -2690,11 +2698,13 @@ * Lookup the faulting address. */ - vm_map_lock_read(map); + if (!lockheld) + vm_map_lock_read(map); #define RETURN(why) \ { \ - vm_map_unlock_read(map); \ + if (!lockheld) \ + vm_map_unlock_read(map); \ return(why); \ } @@ -2730,7 +2740,8 @@ vm_map_t old_map = map; *var_map = map = entry->object.sub_map; - vm_map_unlock_read(old_map); + if (!lockheld) + vm_map_unlock_read(old_map); goto RetryLookup; } Index: vm_map.h =================================================================== RCS file: /home/freebsd/ncvs/src/sys/vm/vm_map.h,v retrieving revision 1.67 diff -u -r1.67 vm_map.h --- vm_map.h 2001/09/12 08:38:11 1.67 +++ vm_map.h 2001/10/04 00:41:44 @@ -71,7 +71,8 @@ #ifndef _VM_MAP_ #define _VM_MAP_ -#include +#include +#include #ifdef MAP_LOCK_DIAGNOSTIC #include @@ -154,7 +155,7 @@ */ struct vm_map { struct vm_map_entry header; /* List of entries */ - struct lock lock; /* Lock for map data */ + struct sx lock; /* Lock for map data */ int nentries; /* Number of entries */ vm_size_t size; /* virtual size */ u_char system_map; /* Am I a system map? */ @@ -216,13 +217,12 @@ #endif void vm_map_lock(vm_map_t map); +int vm_map_trylock(vm_map_t map); void vm_map_unlock(vm_map_t map); void vm_map_lock_read(vm_map_t map); void vm_map_unlock_read(vm_map_t map); int vm_map_lock_upgrade(vm_map_t map); void vm_map_lock_downgrade(vm_map_t map); -void vm_map_set_recursive(vm_map_t map); -void vm_map_clear_recursive(vm_map_t map); vm_offset_t vm_map_min(vm_map_t map); vm_offset_t vm_map_max(vm_map_t map); struct pmap *vm_map_pmap(vm_map_t map); @@ -272,6 +272,8 @@ int vm_map_insert (vm_map_t, vm_object_t, vm_ooffset_t, vm_offset_t, vm_offset_t, vm_prot_t, vm_prot_t, int); int vm_map_lookup (vm_map_t *, vm_offset_t, vm_prot_t, vm_map_entry_t *, vm_object_t *, vm_pindex_t *, vm_prot_t *, boolean_t *); +int vm_map_lookup2 (vm_map_t *, vm_offset_t, vm_prot_t, vm_map_entry_t *, vm_object_t *, + vm_pindex_t *, vm_prot_t *, boolean_t *, boolean_t); void vm_map_lookup_done (vm_map_t, vm_map_entry_t); boolean_t vm_map_lookup_entry (vm_map_t, vm_offset_t, vm_map_entry_t *); int vm_map_pageable (vm_map_t, vm_offset_t, vm_offset_t, boolean_t); Index: vm_pageout.c =================================================================== RCS file: /home/freebsd/ncvs/src/sys/vm/vm_pageout.c,v retrieving revision 1.183 diff -u -r1.183 vm_pageout.c --- vm_pageout.c 2001/09/12 08:38:11 1.183 +++ vm_pageout.c 2001/10/04 00:41:47 @@ -546,9 +546,8 @@ vm_object_t obj, bigobj; GIANT_REQUIRED; - if (lockmgr(&map->lock, LK_EXCLUSIVE | LK_NOWAIT, (void *)0, curthread)) { + if (vm_map_trylock(map)) return; - } bigobj = NULL; Index: vm_zone.c =================================================================== RCS file: /home/freebsd/ncvs/src/sys/vm/vm_zone.c,v retrieving revision 1.48 diff -u -r1.48 vm_zone.c --- vm_zone.c 2001/08/05 03:55:02 1.48 +++ vm_zone.c 2001/10/04 00:41:48 @@ -409,11 +409,12 @@ * map. */ mtx_unlock(&z->zmtx); - if (lockstatus(&kernel_map->lock, NULL)) { + if (vm_map_trylock(kernel_map)) { item = (void *) kmem_malloc(kmem_map, nbytes, M_WAITOK); if (item != NULL) atomic_add_int(&zone_kmem_pages, z->zalloc); } else { + vm_map_unlock(kernel_map); item = (void *) kmem_alloc(kernel_map, nbytes); if (item != NULL) atomic_add_int(&zone_kern_pages, z->zalloc); -- Brian Fundakowski Feldman \ FreeBSD: The Power to Serve! / green@FreeBSD.org `------------------------------' To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Thu Oct 4 2:31:40 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mail6.speakeasy.net (mail6.speakeasy.net [216.254.0.206]) by hub.freebsd.org (Postfix) with ESMTP id 7C79D37B407 for ; Thu, 4 Oct 2001 02:31:32 -0700 (PDT) Received: (qmail 40208 invoked from network); 4 Oct 2001 09:31:31 -0000 Received: from unknown (HELO laptop.baldwin.cx) ([64.81.54.73]) (envelope-sender ) by mail6.speakeasy.net (qmail-ldap-1.03) with SMTP for ; 4 Oct 2001 09:31:31 -0000 Message-ID: X-Mailer: XFMail 1.4.0 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: <200110040105.f9415av15297@green.bikeshed.org> Date: Thu, 04 Oct 2001 02:31:06 -0700 (PDT) From: John Baldwin To: Brian Fundakowski Feldman Subject: RE: sx-ifying src/sys/vm Cc: arch@FreeBSD.org Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 04-Oct-01 Brian Fundakowski Feldman wrote: > I've taken a shot at changing the vm_map locking from using lockmgr() locks > (EAAARGHH...) to sx locks. Already I can tell that it 1. seems to work and > 2. seems to not give me bogus lock ordering messages like lockmgr() used to. > I'd appreciate if people took a look at it and told me of any concerns, > whether it also looks fine to more than just myself, etc. > > I added a few more vm_map locking operations than were there already for the > purpose of removing the hard-coded lockmgr() ops that were in a few places > previously. Instead of having a sx_locked() operation I used > sx_try_xlock(), and instead of recursion I saved the state of the one case > where a lock was attempted to be "recursed" upon, and removed the actual > recursion on the lock. Note that recursion of shared locks is allowed. Also, the name vm_map_lookup2() is really gross. However, if you only recurse on a read lock, then you can get rid of lockheld it seems and remove the vm_map_lookup() ugliness. > Index: vm_fault.c > =================================================================== > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_fault.c,v > retrieving revision 1.125 > diff -u -r1.125 vm_fault.c > --- vm_fault.c 2001/09/12 08:38:11 1.125 > +++ vm_fault.c 2001/10/04 00:41:36 > @@ -213,12 +214,16 @@ > int hardfault; > int faultcount; > struct faultstate fs; > + boolean_t user_wire; > > GIANT_REQUIRED; > > cnt.v_vm_faults++; > hardfault = 0; > + user_wire = (fault_flags & VM_FAULT_WIRE_MASK) == VM_FAULT_USER_WIRE; > + fs.lockheld = user_wire; > > + > RetryFault:; > > /* Extra blank line? > Index: vm_glue.c > =================================================================== > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_glue.c,v > retrieving revision 1.119 > diff -u -r1.119 vm_glue.c > --- vm_glue.c 2001/09/12 08:38:11 1.119 > +++ vm_glue.c 2001/10/04 00:41:37 > @@ -577,9 +577,7 @@ > * data structures there is a > * possible deadlock. > */ > - if (lockmgr(&vm->vm_map.lock, > - LK_EXCLUSIVE | LK_NOWAIT, > - NULL, curthread)) { > + if (vm_map_trylock(&vm->vm_map)) { > vmspace_free(vm); > PROC_UNLOCK(p); > goto nextproc; Hrm, this reads weird. It should be 'if (!vm_map_trylock()) {'. I would make vm_map_trylock() return true on success and false on failure. Same for vm_map_lock_upgrade(). > Index: vm_map.c > =================================================================== > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_map.c,v > retrieving revision 1.207 > diff -u -r1.207 vm_map.c > --- vm_map.c 2001/09/12 08:38:11 1.207 > +++ vm_map.c 2001/10/04 > @@ -403,7 +392,7 @@ > map->first_free = &map->header; > map->hint = &map->header; > map->timestamp = 0; > - lockinit(&map->lock, PVM, "thrd_sleep", 0, LK_NOPAUSE); > + sx_init(&map->lock, "thrd_sleep"); > } > > void Why not a more recognizable name such as "vm_map"? > @@ -1490,7 +1479,6 @@ > estart = entry->start; > > /* First we need to allow map modifications */ > - vm_map_set_recursive(map); > vm_map_lock_downgrade(map); > map->timestamp++; > Old bug: why is the timestamp updated while not holding an exclusive lock? If anything, it should be bumped before the downgrade. Otherwise there isn't a memory barrier there to ensure other slock holders will see the right timestamp on other CPU's. > @@ -1500,14 +1488,12 @@ > entry->wired_count--; > entry->eflags &= ~MAP_ENTRY_USER_WIRED; > > - vm_map_clear_recursive(map); > - vm_map_unlock(map); > + vm_map_unlock_read(map); > > (void) vm_map_user_pageable(map, start, entry->start, TRUE); > return rv; > } > > - vm_map_clear_recursive(map); > if (vm_map_lock_upgrade(map)) { > vm_map_lock(map); > if (vm_map_lookup_entry(map, estart, &entry) Another old bug: why is the lock being modified w/o holding an exclusive lock? Looks like this one is now moot, however. > @@ -2678,6 +2670,22 @@ > vm_prot_t *out_prot, /* OUT */ > boolean_t *wired) /* OUT */ > { > + > + return (vm_map_lookup2(var_map, vaddr, fault_typea, out_entry, > + object, pindex, out_prot, wired, 0)); > +} > + > +int > +vm_map_lookup2(vm_map_t *var_map, /* IN/OUT */ > + vm_offset_t vaddr, > + vm_prot_t fault_typea, > + vm_map_entry_t *out_entry, /* OUT */ > + vm_object_t *object, /* OUT */ > + vm_pindex_t *pindex, /* OUT */ > + vm_prot_t *out_prot, /* OUT */ > + boolean_t *wired, /* OUT */ > + boolean_t lockheld) > +{ > vm_map_entry_t entry; > vm_map_t map = *var_map; > vm_prot_t prot; > @@ -2690,11 +2698,13 @@ > * Lookup the faulting address. > */ > > - vm_map_lock_read(map); > + if (!lockheld) > + vm_map_lock_read(map); > > #define RETURN(why) \ > { \ > - vm_map_unlock_read(map); \ > + if (!lockheld) \ > + vm_map_unlock_read(map); \ > return(why); \ > } > > @@ -2730,7 +2740,8 @@ > vm_map_t old_map = map; > > *var_map = map = entry->object.sub_map; > - vm_map_unlock_read(old_map); > + if (!lockheld) > + vm_map_unlock_read(old_map); > goto RetryLookup; > } > Ok, I don't like the lockheld parameter here. :) It looks like the recursive locks are read locks. Is the problem that you can call this function with an exclusive lock held? If so, I would rather that you add an assertion SX_ASSERT_LOCKED(&map->lock) that the map is locked (either read or write) by the caller. Then vm_map_lookup() can simply assume the map is already locked. > Index: vm_zone.c > =================================================================== > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_zone.c,v > retrieving revision 1.48 > diff -u -r1.48 vm_zone.c > --- vm_zone.c 2001/08/05 03:55:02 1.48 > +++ vm_zone.c 2001/10/04 00:41:48 > @@ -409,11 +409,12 @@ > * map. > */ > mtx_unlock(&z->zmtx); > - if (lockstatus(&kernel_map->lock, NULL)) { > + if (vm_map_trylock(kernel_map)) { > item = (void *) kmem_malloc(kmem_map, nbytes, M_WAITOK); > if (item != NULL) > atomic_add_int(&zone_kmem_pages, z->zalloc); > } else { > + vm_map_unlock(kernel_map); > item = (void *) kmem_alloc(kernel_map, nbytes); > if (item != NULL) > atomic_add_int(&zone_kern_pages, z->zalloc); Question: why does the zone allocator care if the map is locked or not? Also, this has a race: some other thread could easily come in and lock the map in between the vm_map_unlock() and the kmem_alloc(), thus rendering this test useless. This entire test here is not thread safe and needs to be rethought. -- John Baldwin -- http://www.FreeBSD.org/~jhb/ PGP Key: http://www.baldwin.cx/~john/pgpkey.asc "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Thu Oct 4 4:30: 3 2001 Delivered-To: freebsd-arch@freebsd.org Received: from green.bikeshed.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id 3B9C337B403; Thu, 4 Oct 2001 04:29:46 -0700 (PDT) Received: from localhost (green@localhost) by green.bikeshed.org (8.11.4/8.11.1) with ESMTP id f94BTjl31274; Thu, 4 Oct 2001 07:29:45 -0400 (EDT) (envelope-from green@green.bikeshed.org) Message-Id: <200110041129.f94BTjl31274@green.bikeshed.org> X-Mailer: exmh version 2.5 07/13/2001 with nmh-1.0.4 To: John Baldwin Cc: Brian Fundakowski Feldman , arch@FreeBSD.org Subject: Re: sx-ifying src/sys/vm In-Reply-To: Your message of "Thu, 04 Oct 2001 02:31:06 PDT." From: "Brian F. Feldman" Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Thu, 04 Oct 2001 07:29:44 -0400 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG John Baldwin wrote: > > On 04-Oct-01 Brian Fundakowski Feldman wrote: > > I've taken a shot at changing the vm_map locking from using lockmgr() locks > > (EAAARGHH...) to sx locks. Already I can tell that it 1. seems to work and > > 2. seems to not give me bogus lock ordering messages like lockmgr() used to. > > I'd appreciate if people took a look at it and told me of any concerns, > > whether it also looks fine to more than just myself, etc. > > > > I added a few more vm_map locking operations than were there already for the > > purpose of removing the hard-coded lockmgr() ops that were in a few places > > previously. Instead of having a sx_locked() operation I used > > sx_try_xlock(), and instead of recursion I saved the state of the one case > > where a lock was attempted to be "recursed" upon, and removed the actual > > recursion on the lock. > > Note that recursion of shared locks is allowed. Also, the name > vm_map_lookup2() is really gross. However, if you only recurse on a read lock, > then you can get rid of lockheld it seems and remove the vm_map_lookup() > ugliness. > > > Index: vm_fault.c > > =================================================================== > > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_fault.c,v > > retrieving revision 1.125 > > diff -u -r1.125 vm_fault.c > > --- vm_fault.c 2001/09/12 08:38:11 1.125 > > +++ vm_fault.c 2001/10/04 00:41:36 > > @@ -213,12 +214,16 @@ > > int hardfault; > > int faultcount; > > struct faultstate fs; > > + boolean_t user_wire; > > > > GIANT_REQUIRED; > > > > cnt.v_vm_faults++; > > hardfault = 0; > > + user_wire = (fault_flags & VM_FAULT_WIRE_MASK) == VM_FAULT_USER_WIRE; > > + fs.lockheld = user_wire; > > > > + > > RetryFault:; > > > > /* > > Extra blank line? Oops :) > > Index: vm_glue.c > > =================================================================== > > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_glue.c,v > > retrieving revision 1.119 > > diff -u -r1.119 vm_glue.c > > --- vm_glue.c 2001/09/12 08:38:11 1.119 > > +++ vm_glue.c 2001/10/04 00:41:37 > > @@ -577,9 +577,7 @@ > > * data structures there is a > > * possible deadlock. > > */ > > - if (lockmgr(&vm->vm_map.lock, > > - LK_EXCLUSIVE | LK_NOWAIT, > > - NULL, curthread)) { > > + if (vm_map_trylock(&vm->vm_map)) { > > vmspace_free(vm); > > PROC_UNLOCK(p); > > goto nextproc; > > Hrm, this reads weird. It should be 'if (!vm_map_trylock()) {'. I would make > vm_map_trylock() return true on success and false on failure. Same for > vm_map_lock_upgrade(). I suppose, but errno return types are consistent with lockmgr. I also expect them a hell of a lot more than some other kind of int return values. > > Index: vm_map.c > > =================================================================== > > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_map.c,v > > retrieving revision 1.207 > > diff -u -r1.207 vm_map.c > > --- vm_map.c 2001/09/12 08:38:11 1.207 > > +++ vm_map.c 2001/10/04 > > @@ -403,7 +392,7 @@ > > map->first_free = &map->header; > > map->hint = &map->header; > > map->timestamp = 0; > > - lockinit(&map->lock, PVM, "thrd_sleep", 0, LK_NOPAUSE); > > + sx_init(&map->lock, "thrd_sleep"); > > } > > > > void > > Why not a more recognizable name such as "vm_map"? I was wondering that myself. > > @@ -1490,7 +1479,6 @@ > > estart = entry->start; > > > > /* First we need to allow map modifications */ > > - vm_map_set_recursive(map); > > vm_map_lock_downgrade(map); > > map->timestamp++; > > > > Old bug: why is the timestamp updated while not holding an exclusive lock? If > anything, it should be bumped before the downgrade. Otherwise there isn't a > memory barrier there to ensure other slock holders will see the right timestamp > on other CPU's. I don't think the code was designed to make sense in an SMP world. That in mind, I'll go through all vm_map locking-related calls and look for this kind of inconsistency. > > @@ -1500,14 +1488,12 @@ > > entry->wired_count--; > > entry->eflags &= ~MAP_ENTRY_USER_WIRED; > > > > - vm_map_clear_recursive(map); > > - vm_map_unlock(map); > > + vm_map_unlock_read(map); > > > > (void) vm_map_user_pageable(map, start, > entry->start, TRUE); > > return rv; > > } > > > > - vm_map_clear_recursive(map); > > if (vm_map_lock_upgrade(map)) { > > vm_map_lock(map); > > if (vm_map_lookup_entry(map, estart, &entry) > > Another old bug: why is the lock being modified w/o holding an exclusive lock? > Looks like this one is now moot, however. What do you mean exactly? The old way was to get the lockmgr's simplelock, modify the lockmgr itself, then release it. The vm_map's entries are being modified with an exclusive lock there, if I'm not mistaken... That may be an implicit dependency on Giant (I doubt it) or just a bug that the author didn't catch due to the following vm_map_unlock() not explicitly documenting whether the map was to be read-locked or write-locked at that point. > > @@ -2678,6 +2670,22 @@ > > vm_prot_t *out_prot, /* OUT */ > > boolean_t *wired) /* OUT */ > > { > > + > > + return (vm_map_lookup2(var_map, vaddr, fault_typea, out_entry, > > + object, pindex, out_prot, wired, 0)); > > +} > > + > > +int > > +vm_map_lookup2(vm_map_t *var_map, /* IN/OUT */ > > + vm_offset_t vaddr, > > + vm_prot_t fault_typea, > > + vm_map_entry_t *out_entry, /* OUT */ > > + vm_object_t *object, /* OUT */ > > + vm_pindex_t *pindex, /* OUT */ > > + vm_prot_t *out_prot, /* OUT */ > > + boolean_t *wired, /* OUT */ > > + boolean_t lockheld) > > +{ > > vm_map_entry_t entry; > > vm_map_t map = *var_map; > > vm_prot_t prot; > > @@ -2690,11 +2698,13 @@ > > * Lookup the faulting address. > > */ > > > > - vm_map_lock_read(map); > > + if (!lockheld) > > + vm_map_lock_read(map); > > > > #define RETURN(why) \ > > { \ > > - vm_map_unlock_read(map); \ > > + if (!lockheld) \ > > + vm_map_unlock_read(map); \ > > return(why); \ > > } > > > > @@ -2730,7 +2740,8 @@ > > vm_map_t old_map = map; > > > > *var_map = map = entry->object.sub_map; > > - vm_map_unlock_read(old_map); > > + if (!lockheld) > > + vm_map_unlock_read(old_map); > > goto RetryLookup; > > } > > > > Ok, I don't like the lockheld parameter here. :) It looks like the recursive > locks are read locks. Is the problem that you can call this function with an > exclusive lock held? If so, I would rather that you add an assertion > SX_ASSERT_LOCKED(&map->lock) that the map is locked (either read or write) by > the caller. Then vm_map_lookup() can simply assume the map is already locked. vm_map_lookup() calls vm_map_lock_upgrade(), and in this case it would call vm_map_lock_upgrade() while already having two shared locks, thus making the sx lock have an sx_cnt of 2 and the vm_map_lock_upgrade() would always subsequently fail in sx_try_upgrade, would it not? > > Index: vm_zone.c > > =================================================================== > > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_zone.c,v > > retrieving revision 1.48 > > diff -u -r1.48 vm_zone.c > > --- vm_zone.c 2001/08/05 03:55:02 1.48 > > +++ vm_zone.c 2001/10/04 00:41:48 > > @@ -409,11 +409,12 @@ > > * map. > > */ > > mtx_unlock(&z->zmtx); > > - if (lockstatus(&kernel_map->lock, NULL)) { > > + if (vm_map_trylock(kernel_map)) { > > item = (void *) kmem_malloc(kmem_map, nbytes, M_WAITOK); > > if (item != NULL) > > atomic_add_int(&zone_kmem_pages, z->zalloc); > > } else { > > + vm_map_unlock(kernel_map); > > item = (void *) kmem_alloc(kernel_map, nbytes); > > if (item != NULL) > > atomic_add_int(&zone_kern_pages, z->zalloc); > > Question: why does the zone allocator care if the map is locked or not? Also, > this has a race: some other thread could easily come in and lock the map in > between the vm_map_unlock() and the kmem_alloc(), thus rendering this test > useless. This entire test here is not thread safe and needs to be rethought. I believe the code is meaning to check lockstatus() is LK_EXCLOTHER or LK_SHARED, and it would be broken if LK_EXCLUSIVE were returned here :) I really was wondering about this. If you can't have faults in an interrupt context (well, you can't use lockmgr from an interrupt context that I know either...), then in non-SMP world this would be just fine, wouldn't it? Anyway, it would be easy to fix if kmem_alloc() didn't xlock the map itself, which it does. It would be easy to make it do that by generating, say, a kmem_alloc2() ;) -- Brian Fundakowski Feldman \ FreeBSD: The Power to Serve! / green@FreeBSD.org `------------------------------' To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Thu Oct 4 10:27:23 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id B283537B405; Thu, 4 Oct 2001 10:27:02 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id TAA52914; Thu, 4 Oct 2001 19:27:01 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: arch@freebsd.org Subject: Removing ptrace(2)'s dependency on procfs(5) From: Dag-Erling Smorgrav Date: 04 Oct 2001 19:27:00 +0200 Message-ID: Lines: 51 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG [Bcc:ed to -security, please followup on -arch] The ptrace(2) syscall, which is mainly used by gdb(1), implements some of its functionality by faking up a struct uio and making calls into the guts of procfs(5). This is why four of the source files that make up procfs(5) are listed as "standard" rather than as "optional procfs" in src/sys/conf/files. The funny thing is that there's no reason for the ptrace(2) code to call any procfs(5) code, since the functions it calls are actually (mostly) wrappers around MD code, with some extra error checking. The errors they check for are of the kind that Can't Happen[tm] when these wrappers are called from ptrace(2) because ptrace(2) already checks for them before calling the procfs(5) code. For instance, all procfs_domem() does is check that uio->uio_resid is non-zero (ptrace() sets it to sizeof(int)) and that the requesting process is allowed to debug the target process (already checked by ptrace()), and then pass its arguments unmodified to procfs_rwmem(). What I propose to do is: - move procfs_rwmem() from src/sys/fs/procfs/procfs_mem.c into src/sys/kern/sys_process.c or some other convenient location where both ptrace(2) and procfs(5) can access it (and also move its prototype to a convenient header file). - rewrite the remaining cases (PT_{GET,SET}{,DB,FP}REGS) to call procfs_{read,write}_regs() (which is implemented in each port's procfs_machdep.c) directly, instead of calling procfs_do*(). - make the permission checks at the top of ptrace(2) slightly more aggressive (immediately return EINVAL if the target process is a system process; immediately return EPERM if p_candebug() returns non-zero and an operation other that PT_TRACE_ME was requested). This will slightly reduce the size (and speed up the build) of a procfs(5)-less kernel. I would also like to implement a kernel option named NO_DEBUGGING (or something similar) which replaces ptrace(2) and the MD parts of procfs(5) with stubs that simply return EINVAL or EPERM, thus making it impossible to use gdb(1), truss(1) and similar tools; I will also change procfs(5) to not list debugging-related files (ctl, regs etc.) when loaded by a kernel that was built with NO_DEBUGGING. Comments? Flames? DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Thu Oct 4 10:31:27 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mail11.speakeasy.net (mail11.speakeasy.net [216.254.0.211]) by hub.freebsd.org (Postfix) with ESMTP id E117C37B405 for ; Thu, 4 Oct 2001 10:31:08 -0700 (PDT) Received: (qmail 96449 invoked from network); 4 Oct 2001 17:31:07 -0000 Received: from unknown (HELO laptop.baldwin.cx) ([64.81.54.73]) (envelope-sender ) by mail11.speakeasy.net (qmail-ldap-1.03) with SMTP for ; 4 Oct 2001 17:31:07 -0000 Message-ID: X-Mailer: XFMail 1.4.0 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: <200110041129.f94BTjl31274@green.bikeshed.org> Date: Thu, 04 Oct 2001 10:30:43 -0700 (PDT) From: John Baldwin To: "Brian F. Feldman" Subject: Re: sx-ifying src/sys/vm Cc: arch@FreeBSD.org Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 04-Oct-01 Brian F. Feldman wrote: >> > Index: vm_glue.c >> > =================================================================== >> > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_glue.c,v >> > retrieving revision 1.119 >> > diff -u -r1.119 vm_glue.c >> > --- vm_glue.c 2001/09/12 08:38:11 1.119 >> > +++ vm_glue.c 2001/10/04 00:41:37 >> > @@ -577,9 +577,7 @@ >> > * data structures there is a >> > * possible deadlock. >> > */ >> > - if (lockmgr(&vm->vm_map.lock, >> > - LK_EXCLUSIVE | LK_NOWAIT, >> > - NULL, curthread)) { >> > + if (vm_map_trylock(&vm->vm_map)) { >> > vmspace_free(vm); >> > PROC_UNLOCK(p); >> > goto nextproc; >> >> Hrm, this reads weird. It should be 'if (!vm_map_trylock()) {'. I would >> make >> vm_map_trylock() return true on success and false on failure. Same for >> vm_map_lock_upgrade(). > > I suppose, but errno return types are consistent with lockmgr. I also > expect them a hell of a lot more than some other kind of int return values. But you aren't _using_ that return value anywhere, and it makes the code more confusing to read. >> Another old bug: why is the lock being modified w/o holding an exclusive >> lock? >> Looks like this one is now moot, however. > > What do you mean exactly? The old way was to get the lockmgr's simplelock, > modify the lockmgr itself, then release it. Ah, the simplelock handles it then. >> Ok, I don't like the lockheld parameter here. :) It looks like the >> recursive >> locks are read locks. Is the problem that you can call this function with >> an >> exclusive lock held? If so, I would rather that you add an assertion >> SX_ASSERT_LOCKED(&map->lock) that the map is locked (either read or write) >> by >> the caller. Then vm_map_lookup() can simply assume the map is already >> locked. > > vm_map_lookup() calls vm_map_lock_upgrade(), and in this case it would call > vm_map_lock_upgrade() while already having two shared locks, thus making the > sx lock have an sx_cnt of 2 and the vm_map_lock_upgrade() would always > subsequently fail in sx_try_upgrade, would it not? Ok, then add an assertion SX_ASSERT_SLOCKED(&map->lock) and require callers to call vm_map_lookup() with the map already share locked and have vm_map_lookup() just do upgrade/downgrade but not actually get a slock itself. How did this work with lockmgr() btw? I thought it didn't allow an upgrade of a recursed share lock either. >> > Index: vm_zone.c >> > =================================================================== >> > RCS file: /home/freebsd/ncvs/src/sys/vm/vm_zone.c,v >> > retrieving revision 1.48 >> > diff -u -r1.48 vm_zone.c >> > --- vm_zone.c 2001/08/05 03:55:02 1.48 >> > +++ vm_zone.c 2001/10/04 00:41:48 >> > @@ -409,11 +409,12 @@ >> > * map. >> > */ >> > mtx_unlock(&z->zmtx); >> > - if (lockstatus(&kernel_map->lock, NULL)) { >> > + if (vm_map_trylock(kernel_map)) { >> > item = (void *) kmem_malloc(kmem_map, nbytes, >> > M_WAITOK); >> > if (item != NULL) >> > atomic_add_int(&zone_kmem_pages, z->zalloc); >> > } else { >> > + vm_map_unlock(kernel_map); >> > item = (void *) kmem_alloc(kernel_map, nbytes); >> > if (item != NULL) >> > atomic_add_int(&zone_kern_pages, z->zalloc); >> >> Question: why does the zone allocator care if the map is locked or not? >> Also, >> this has a race: some other thread could easily come in and lock the map in >> between the vm_map_unlock() and the kmem_alloc(), thus rendering this test >> useless. This entire test here is not thread safe and needs to be >> rethought. > > I believe the code is meaning to check lockstatus() is LK_EXCLOTHER or > LK_SHARED, and it would be broken if LK_EXCLUSIVE were returned here :) The comment above explicitly mentions that it is avoiding recursion, which menas it does indeeed want to do this in the LK_EXCLUSIVE case. The problem is if the caller has xlock'd the kernel_map, you don't know if it is valid or not. > I really was wondering about this. If you can't have faults in an interrupt > context (well, you can't use lockmgr from an interrupt context that I know > either...), then in non-SMP world this would be just fine, wouldn't it? You can have a fault in an interrupt context. Probably it's a fatal one like a NULL pointer deref, but you can still have it. However, interrupts aren't in the same context as the interrupted process in -current anymore, so it's less of an issue. > Anyway, it would be easy to fix if kmem_alloc() didn't xlock the map itself, > which it does. It would be easy to make it do that by generating, say, a > kmem_alloc2() ;) Ugh, can't you pick a better function name? :-P You could also just add a kmem_alloc_try() that used a trylock internally and returned NULL if it couldn't get the lock. Then you could code this like so: item = (void *) kmem_alloc(kernel_map, ...); if (item != NULL) atomic_add_int(...); else { item = (void *) kmem_malloc(kmem_map, ...) if (item != NULL) atomic_add_int(...); } Apparently the code can be called with an xlock already held on the kernel_map, in which case we want to fail. I'm not sure why the comment cares if another process has the map locked. It shouldn't. The lockstatus() really should have only failed in the LK_EXCLUSIVE case, and tried to do an upgrade in the LK_SHARED case and just blocked on the lock in the LK_EXCLOTHER case. -- John Baldwin -- http://www.FreeBSD.org/~jhb/ PGP Key: http://www.baldwin.cx/~john/pgpkey.asc "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Thu Oct 4 13:13:17 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 1C2DF37B409 for ; Thu, 4 Oct 2001 13:13:13 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id WAA53790; Thu, 4 Oct 2001 22:13:07 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: From: Dag-Erling Smorgrav Date: 04 Oct 2001 22:13:07 +0200 In-Reply-To: Message-ID: Lines: 5 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG There's a patch up on . DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Thu Oct 4 16: 1:58 2001 Delivered-To: freebsd-arch@freebsd.org Received: from peter3.wemm.org (c1315225-a.plstn1.sfba.home.com [24.14.150.180]) by hub.freebsd.org (Postfix) with ESMTP id A935637B403 for ; Thu, 4 Oct 2001 16:01:54 -0700 (PDT) Received: from overcee.netplex.com.au (overcee.wemm.org [10.0.0.3]) by peter3.wemm.org (8.11.0/8.11.0) with ESMTP id f94N1sM57997 for ; Thu, 4 Oct 2001 16:01:54 -0700 (PDT) (envelope-from peter@wemm.org) Received: from wemm.org (localhost [127.0.0.1]) by overcee.netplex.com.au (Postfix) with ESMTP id 4A0D63809; Thu, 4 Oct 2001 16:01:54 -0700 (PDT) (envelope-from peter@wemm.org) X-Mailer: exmh version 2.3.1 01/18/2001 with nmh-1.0.4 To: Dag-Erling Smorgrav Cc: arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Date: Thu, 04 Oct 2001 16:01:54 -0700 From: Peter Wemm Message-Id: <20011004230154.4A0D63809@overcee.netplex.com.au> Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Dag-Erling Smorgrav wrote: > - move procfs_rwmem() from src/sys/fs/procfs/procfs_mem.c into > src/sys/kern/sys_process.c or some other convenient location where > both ptrace(2) and procfs(5) can access it (and also move its > prototype to a convenient header file). It seems to be mostly VM code, perhaps it should be somewhere in vm/*, perhaps vm/vm_glue.c ? > - rewrite the remaining cases (PT_{GET,SET}{,DB,FP}REGS) to call > procfs_{read,write}_regs() (which is implemented in each port's > procfs_machdep.c) directly, instead of calling procfs_do*(). Hmm. We have things like this: int procfs_read_regs(td, regs) struct thread *td; struct reg *regs; { PROCFS_ACTION(fill_regs(td, regs)); } int procfs_sstep(td) struct thread *td; { PROCFS_ACTION(ptrace_single_step(td)); } Would it not make more sense to just make ptrace_{read|write}_*regs() in machdep.c rather than have ptrace go via procfs functions and back to machdep.c? This doesn't have to be done all at once. The patch that you posted after this one looks like a good start so far. Cheers, -Peter -- Peter Wemm - peter@FreeBSD.org; peter@yahoo-inc.com; peter@netplex.com.au "All of this is for nothing if we don't go to the stars" - JMS/B5 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Thu Oct 4 16:11:47 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 2947037B403 for ; Thu, 4 Oct 2001 16:11:43 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id BAA54399; Fri, 5 Oct 2001 01:11:39 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Peter Wemm Cc: arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: <20011004230154.4A0D63809@overcee.netplex.com.au> From: Dag-Erling Smorgrav Date: 05 Oct 2001 01:11:38 +0200 In-Reply-To: <20011004230154.4A0D63809@overcee.netplex.com.au> Message-ID: Lines: 33 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Peter Wemm writes: > Dag-Erling Smorgrav wrote: > > - move procfs_rwmem() from src/sys/fs/procfs/procfs_mem.c into > > src/sys/kern/sys_process.c or some other convenient location where > > both ptrace(2) and procfs(5) can access it (and also move its > > prototype to a convenient header file). > It seems to be mostly VM code, perhaps it should be somewhere in vm/*, > perhaps vm/vm_glue.c ? procfs_rwmem() was originally derived from code which still resides (#if 0'd out) in sys_process.c. That's why I felt it was the most logical place to move it to. > Would it not make more sense to just make ptrace_{read|write}_*regs() > in machdep.c rather than have ptrace go via procfs functions and back to > machdep.c? That's exactly what I'm saying. In case you're confused, the PROCFS_ACTION() stuff in procfs_machdep.c has nothing to do with procfs, it's just a poorly-named macro that evaluates its arguments and does some error checking. > This doesn't have to be done all at once. The patch that you posted after > this one looks like a good start so far. Yep - at some point the functions should be renamed, and the prototypes should probably go into instead of the bogus I added. DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Thu Oct 4 16:31:34 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 92FF137B403 for ; Thu, 4 Oct 2001 16:31:28 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id BAA54514; Fri, 5 Oct 2001 01:31:24 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Peter Wemm Cc: arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: <20011004230154.4A0D63809@overcee.netplex.com.au> From: Dag-Erling Smorgrav Date: 05 Oct 2001 01:31:23 +0200 In-Reply-To: Message-ID: Lines: 13 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Dag-Erling Smorgrav writes: > Yep - at some point the functions should be renamed, and the > prototypes should probably go into instead of the bogus > I added. I've put up a new patch that places the prototypes in ptrace.h rather than add a new header: http://people.freebsd.org/~des/software/ptrace-20011005.diff DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Thu Oct 4 21:48:33 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 459AB37B401 for ; Thu, 4 Oct 2001 21:48:30 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id OAA10541; Fri, 5 Oct 2001 14:48:08 +1000 Date: Fri, 5 Oct 2001 14:47:28 +1000 (EST) From: Bruce Evans X-X-Sender: To: Dag-Erling Smorgrav Cc: Peter Wemm , Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Message-ID: <20011005141014.W12962-100000@delplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 5 Oct 2001, Dag-Erling Smorgrav wrote: > Peter Wemm writes: > > Dag-Erling Smorgrav wrote: > > > - move procfs_rwmem() from src/sys/fs/procfs/procfs_mem.c into > > > src/sys/kern/sys_process.c or some other convenient location where > > > both ptrace(2) and procfs(5) can access it (and also move its > > > prototype to a convenient header file). > > It seems to be mostly VM code, perhaps it should be somewhere in vm/*, > > perhaps vm/vm_glue.c ? > > procfs_rwmem() was originally derived from code which still resides > (#if 0'd out) in sys_process.c. That's why I felt it was the most > logical place to move it to. This may have happened before the dawn of history, but 4.4BSD-Lite has a full procfs_rwmem() and only stubs in sys_process.c, so FreeBSD certainly didn't derive procfs_rwmem() from sys_process.c. History shows that sys_process.c once used pread(), but peter changed it to use procfs in rev.1.21. It's more likely that pread() was derived from procfs_rwmem() than vice versa. > > Would it not make more sense to just make ptrace_{read|write}_*regs() > > in machdep.c rather than have ptrace go via procfs functions and back to > > machdep.c? > > That's exactly what I'm saying. > > In case you're confused, the PROCFS_ACTION() stuff in procfs_machdep.c > has nothing to do with procfs, it's just a poorly-named macro that > evaluates its arguments and does some error checking. The error checking is bogus too. It checks that PHOLD() actually worked. This is currently necessary because PHOLD() is broken. In fact, all of procfs_machdep.c is bogus. The functions in it just call functions in machdep.c, so procfs_machdep.c is MI, modulo the functions in machdep.c not having gratuitously MD names. The functions in machdep.c should have been inline in procfs_machdep.c, as in NetBSD. This is not quite right in your reorganization. The functions are needed by both procfs and ptrace, so they shouldn't be in procfs_machdep.c or have names beginning with procfs. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Fri Oct 5 3: 4:39 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id EC8A937B405 for ; Fri, 5 Oct 2001 03:04:34 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id MAA56951; Fri, 5 Oct 2001 12:04:22 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Bruce Evans Cc: Peter Wemm , Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: <20011005141014.W12962-100000@delplex.bde.org> From: Dag-Erling Smorgrav Date: 05 Oct 2001 12:04:21 +0200 In-Reply-To: <20011005141014.W12962-100000@delplex.bde.org> Message-ID: Lines: 27 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Bruce Evans writes: > On 5 Oct 2001, Dag-Erling Smorgrav wrote: > > procfs_rwmem() was originally derived from code which still resides > > (#if 0'd out) in sys_process.c. That's why I felt it was the most > > logical place to move it to. > This may have happened before the dawn of history, but 4.4BSD-Lite has > a full procfs_rwmem() and only stubs in sys_process.c, so FreeBSD > certainly didn't derive procfs_rwmem() from sys_process.c. History > shows that sys_process.c once used pread(), but peter changed it to > use procfs in rev.1.21. It's more likely that pread() was derived > from procfs_rwmem() than vice versa. The comments in the code indicate that procfs_rwmem() was derived by merging pread() and pwrite(), not the other way around - and there are a lot of similarities between them. > This is not quite right in your reorganization. The functions are needed > by both procfs and ptrace, so they shouldn't be in procfs_machdep.c or > have names beginning with procfs. I didn't pick that location or those names. I'll move and rename these functions later, when I don't have sixty or seventy uncommitted patches in two different and partially conflicting source trees. DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Fri Oct 5 6:55:55 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 179AE37B401 for ; Fri, 5 Oct 2001 06:55:50 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id PAA57962; Fri, 5 Oct 2001 15:55:40 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Peter Wemm Cc: arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: <20011004230154.4A0D63809@overcee.netplex.com.au> From: Dag-Erling Smorgrav Date: 05 Oct 2001 15:55:39 +0200 In-Reply-To: Message-ID: Lines: 12 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Dag-Erling Smorgrav writes: > I've put up a new patch that places the prototypes in ptrace.h rather > than add a new header: I left one instance of #include in, so this patch wouldn't build. The correct (and tested) patch is: http://people.freebsd.org/~des/software/ptrace-20011005b.diff DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Fri Oct 5 20:14:10 2001 Delivered-To: freebsd-arch@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 1677B37B407 for ; Fri, 5 Oct 2001 20:14:07 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.11.6/8.11.5) with SMTP id f963DkB60137; Fri, 5 Oct 2001 23:13:46 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Fri, 5 Oct 2001 23:13:45 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: Dag-Erling Smorgrav Cc: Peter Wemm , arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG I've already delivered some comments to you out-of-band, but here are a couple more: (1) Actually, this is a duplicate of an out-of-band one: using procfs_rwmem() as a function name in sys_process.c still jibes: are you sure you don't want to rename it now rather than waiting? :-) (2) For the security check: + if (uap->req != PT_TRACE_ME && (error = p_candebug(curp, p))) { + PROC_UNLOCK(p); + return (error); + } Instead, modify p_candebug() to allow debugging of p1 by p1 always. Structuring the P_SYSTEM check that way is fine, as that's a syntax check, but since this case exempts the security check if it's PT_TRACE_ME, I'd rather we modify the security check. Note that one benefit to doing it this way is that if the admin disables debugging globally using the existing policy sysctl, it also disables it for the current process. Otherwise, looks good to me. No doubt once it's committed, there will be some further tweaks, but this is a cleanup I'm very happy to see happen. Thanks! Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services On 5 Oct 2001, Dag-Erling Smorgrav wrote: > Dag-Erling Smorgrav writes: > > I've put up a new patch that places the prototypes in ptrace.h rather > > than add a new header: > > I left one instance of #include in, so this patch > wouldn't build. The correct (and tested) patch is: > > http://people.freebsd.org/~des/software/ptrace-20011005b.diff > > DES > -- > Dag-Erling Smorgrav - des@ofug.org > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-arch" in the body of the message > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 1:34:23 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 474DD37B406; Sat, 6 Oct 2001 01:34:18 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id KAA63859; Sat, 6 Oct 2001 10:34:17 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Robert Watson Cc: Peter Wemm , arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: From: Dag-Erling Smorgrav Date: 06 Oct 2001 10:34:16 +0200 In-Reply-To: Message-ID: Lines: 20 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Robert Watson writes: > (1) Actually, this is a duplicate of an out-of-band one: using > procfs_rwmem() as a function name in sys_process.c still jibes: are > you sure you don't want to rename it now rather than waiting? :-) How does ptrace_rwmem() sound? > Instead, modify p_candebug() to allow debugging of p1 by p1 always. > Structuring the P_SYSTEM check that way is fine, as that's a syntax > check, but since this case exempts the security check if it's > PT_TRACE_ME, I'd rather we modify the security check. Note that one > benefit to doing it this way is that if the admin disables debugging > globally using the existing policy sysctl, it also disables it for the > current process. Sounds reasonable. DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 1:40:58 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id C4A6237B408; Sat, 6 Oct 2001 01:40:55 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id KAA63886; Sat, 6 Oct 2001 10:40:54 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Robert Watson Cc: Peter Wemm , arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: From: Dag-Erling Smorgrav Date: 06 Oct 2001 10:40:54 +0200 In-Reply-To: Message-ID: Lines: 10 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Robert Watson writes: > Instead, modify p_candebug() to allow debugging of p1 by p1 always. Should I also change p_candebug() to always deny the request if p2 is a system process? That will save quite a lot of checks in ptrace() and procfs, and possibly some other places as well. DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 5:27:37 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 69E8737B403 for ; Sat, 6 Oct 2001 05:27:34 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id WAA32096; Sat, 6 Oct 2001 22:27:25 +1000 Date: Sat, 6 Oct 2001 22:26:45 +1000 (EST) From: Bruce Evans X-X-Sender: To: Dag-Erling Smorgrav Cc: Peter Wemm , Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Message-ID: <20011006221156.X824-100000@delplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 5 Oct 2001, Dag-Erling Smorgrav wrote: > Dag-Erling Smorgrav writes: > > I've put up a new patch that places the prototypes in ptrace.h rather > > than add a new header: They don't belong in ptrace.h either, since they are used by both procfs and ptrace. > I left one instance of #include in, so this patch > wouldn't build. The correct (and tested) patch is: > > http://people.freebsd.org/~des/software/ptrace-20011005b.diff Please include things that you want reviewed in mail unless they are large (100K+ or so). I noticed the following bugs: - PHOLD()/PRELE() is now missing from the register access functions of ptrace(). This makes the bogus EIO error in PROCFS_ACTION() much less unlikely. - there is lots of gratuitous breakage of K&R support. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 5:34:45 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 26F6437B403; Sat, 6 Oct 2001 05:34:42 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id WAA32447; Sat, 6 Oct 2001 22:34:37 +1000 Date: Sat, 6 Oct 2001 22:33:56 +1000 (EST) From: Bruce Evans X-X-Sender: To: Dag-Erling Smorgrav Cc: Robert Watson , Peter Wemm , Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Message-ID: <20011006222940.Y824-100000@delplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 6 Oct 2001, Dag-Erling Smorgrav wrote: > Robert Watson writes: > > (1) Actually, this is a duplicate of an out-of-band one: using > > procfs_rwmem() as a function name in sys_process.c still jibes: are > > you sure you don't want to rename it now rather than waiting? :-) > > How does ptrace_rwmem() sound? Slightly worse :-). proc_rwmem() is the best I can think of now. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 6:22:46 2001 Delivered-To: freebsd-arch@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 97EF537B409 for ; Sat, 6 Oct 2001 06:22:42 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.11.6/8.11.5) with SMTP id f96DMAB66690; Sat, 6 Oct 2001 09:22:10 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Sat, 6 Oct 2001 09:22:10 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: Dag-Erling Smorgrav Cc: Peter Wemm , arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 6 Oct 2001, Dag-Erling Smorgrav wrote: > Robert Watson writes: > > Instead, modify p_candebug() to allow debugging of p1 by p1 always. > > Should I also change p_candebug() to always deny the request if p2 is a > system process? That will save quite a lot of checks in ptrace() and > procfs, and possibly some other places as well. Hmm. An interesting question. As I see it, there would be some upsides and some downsides. An upside would relate to process visibility issues: for example, jail() does not permit enclosed processes to see system processes. If the P_SYSTEM check is first, and returns (EINVAL), then a jailed process can enumerate the system process space. Not a huge risk, but not quite in keeping with the intent of p_cansee(). Another choice is to put the check in p_candebug(). However, p_candebug() currently encapsulates security checks rather than validity checks--the inability to debug system processes comes from two components: first, the calling process may not have permission, but second, it's also just not possible :-). Rather than teach p_candebug() about the full scope of the various debugging mechanisms it's supporting, I'd rather have EINVAL checks in the caller. Another reason to do this is that there may potentially be other EINVAL checks depending on the mechanism: for example, if we were to start supporting remote debugging of processes, we might permit reading/writing of memory and registers, but not certain types of mappings. Another possibility would be to reorder the checks of p_cansignal() in ptrace(). I.e., the following: p = pfind(target); if (!p) return (ESRCH) error = p_candebug(curp, p); if (error) return (error); if (p->p_flags & P_SYSTEM) return (EINVAL); This would allow visibility and permission scoping to happen first, since the target is a valid "process", but for ptrace (and other calls) to reject the request if they discover they cannot function in the manner. I think right now I have a preference for the last of these, but I'm willing to be swayed in other directions if that seems useful. Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 6:32:11 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id DB05637B407 for ; Sat, 6 Oct 2001 06:32:07 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id PAA64794; Sat, 6 Oct 2001 15:31:53 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Bruce Evans Cc: Peter Wemm , Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: <20011006221156.X824-100000@delplex.bde.org> From: Dag-Erling Smorgrav Date: 06 Oct 2001 15:31:52 +0200 In-Reply-To: <20011006221156.X824-100000@delplex.bde.org> Message-ID: Lines: 32 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Bruce Evans writes: > I noticed the following bugs: > - PHOLD()/PRELE() is now missing from the register access functions of > ptrace(). This makes the bogus EIO error in PROCFS_ACTION() much less > unlikely. Aieee, thanks for catching that before I committed! BTW, I'm intrigued by the following bit of code in kern_sig.c: do { mtx_lock_spin(&sched_lock); stop(p); PROC_UNLOCK_NOSWITCH(p); DROP_GIANT_NOSWITCH(); p->p_stats->p_ru.ru_nivcsw++; mi_switch(); mtx_unlock_spin(&sched_lock); PICKUP_GIANT(); PROC_LOCK(p); } while (!trace_req(p) && p->p_flag & P_TRACED); It's the only reference to trace_req() (defined in sys_process.c) in the entire tree, and trace_req() has been a constant since revision 1.1 of sys_process.c, and true since revision 1.4 (1994/08/08), so this loop will always execute exactly once. Is there a good reason not to simply axe trace_req() and unroll the loop? DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 6:38: 9 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 5515A37B401; Sat, 6 Oct 2001 06:38:03 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id PAA64818; Sat, 6 Oct 2001 15:38:02 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Robert Watson Cc: Peter Wemm , arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: From: Dag-Erling Smorgrav Date: 06 Oct 2001 15:38:01 +0200 In-Reply-To: Message-ID: Lines: 21 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Robert Watson writes: > On 6 Oct 2001, Dag-Erling Smorgrav wrote: > > Should I also change p_candebug() to always deny the request if p2 is a > > system process? That will save quite a lot of checks in ptrace() and > > procfs, and possibly some other places as well. > Hmm. An interesting question. [...] > > If the P_SYSTEM check is first, and returns (EINVAL), then a jailed > process can enumerate the system process space. Not a huge risk, but not > quite in keeping with the intent of p_cansee(). > > Another choice is to put the check in p_candebug(). [...] I'm confused - I think you misread my question; I was suggesting adding the P_SYSTEM check to p_candebug(), not p_cansee(). If you did not misread my question, you'll have to clarify what you meant in the above three paragraphs :) DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 6:41:26 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id CEFFE37B403 for ; Sat, 6 Oct 2001 06:41:23 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id PAA64837; Sat, 6 Oct 2001 15:41:19 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Bruce Evans Cc: Peter Wemm , Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: <20011006221156.X824-100000@delplex.bde.org> From: Dag-Erling Smorgrav Date: 06 Oct 2001 15:41:19 +0200 In-Reply-To: <20011006221156.X824-100000@delplex.bde.org> Message-ID: Lines: 13 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Bruce Evans writes: > On 5 Oct 2001, Dag-Erling Smorgrav wrote: > > Dag-Erling Smorgrav writes: > > > I've put up a new patch that places the prototypes in ptrace.h rather > > > than add a new header: > They don't belong in ptrace.h either, since they are used by both procfs > and ptrace. then? DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 6:53:48 2001 Delivered-To: freebsd-arch@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id F07DC37B409 for ; Sat, 6 Oct 2001 06:53:43 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.11.6/8.11.5) with SMTP id f96DrNB67051; Sat, 6 Oct 2001 09:53:23 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Sat, 6 Oct 2001 09:53:23 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: Dag-Erling Smorgrav Cc: Peter Wemm , arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 6 Oct 2001, Dag-Erling Smorgrav wrote: > Robert Watson writes: > > On 6 Oct 2001, Dag-Erling Smorgrav wrote: > > > Should I also change p_candebug() to always deny the request if p2 is a > > > system process? That will save quite a lot of checks in ptrace() and > > > procfs, and possibly some other places as well. > > Hmm. An interesting question. [...] > > > > If the P_SYSTEM check is first, and returns (EINVAL), then a jailed > > process can enumerate the system process space. Not a huge risk, but not > > quite in keeping with the intent of p_cansee(). > > > > Another choice is to put the check in p_candebug(). [...] > > I'm confused - I think you misread my question; I was suggesting adding > the P_SYSTEM check to p_candebug(), not p_cansee(). If you did not > misread my question, you'll have to clarify what you meant in the above > three paragraphs :) Well, I guess the decision I was trying to look at was: (1) Is it a global security policy that debugging primitives may never be applied to kernel processes. (2) Is it a syntactic property of the debugging primitive that it *cannot* be applied to kernel processes. I'm leaning towards (2): debugging simply doesn't apply, and should be checked for by the routines. If it were a security check, that might imply that you could change the policy to allow such debugging, which you can't. Since we would still like ESRCH to be returned for processes in jail when attempting to attach to system processes, that means that the security check should go first, and the P_SYSTEM check should go second. Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 7: 1:41 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 47CCC37B405; Sat, 6 Oct 2001 07:01:37 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id QAA64895; Sat, 6 Oct 2001 16:01:36 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Robert Watson Cc: Peter Wemm , arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: From: Dag-Erling Smorgrav Date: 06 Oct 2001 16:01:35 +0200 In-Reply-To: Message-ID: Lines: 41 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Robert Watson writes: > I'm leaning towards (2): debugging simply doesn't apply, and should be > checked for by the routines. If it were a security check, that might > imply that you could change the policy to allow such debugging, which you > can't. Since we would still like ESRCH to be returned for processes in > jail when attempting to attach to system processes, that means that the > security check should go first, and the P_SYSTEM check should go second. Sure, but doesn't prison_check() take care of ESRCH? I can add the P_SYSTEM check *after* the prison_check() call in p_candebug(): Index: kern_prot.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_prot.c,v retrieving revision 1.107 diff -u -u -r1.107 kern_prot.c --- kern_prot.c 26 Sep 2001 20:41:48 -0000 1.107 +++ kern_prot.c 6 Oct 2001 13:56:56 -0000 @@ -1534,9 +1534,15 @@ { int error; + if (p1 == p2) + return (0); + if ((error = prison_check(p1->p_ucred, p2->p_ucred))) return (error); + if ((p2->p_flag & P_SYSTEM) != 0) + return (EINVAL); + /* * Not owned by you, has done setuid (unless you're root). * XXX add a CAP_SYS_PTRACE here? This way, when the target process is a system process, p_candebug() will return ESRCH if the caller is jailed and EINVAL if it isn't. DES (kernel hacking with a hangover) -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 7:50:53 2001 Delivered-To: freebsd-arch@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 0768837B415 for ; Sat, 6 Oct 2001 07:50:47 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.11.6/8.11.5) with SMTP id f96EoJB67539; Sat, 6 Oct 2001 10:50:20 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Sat, 6 Oct 2001 10:50:19 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: Dag-Erling Smorgrav Cc: Peter Wemm , arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 6 Oct 2001, Dag-Erling Smorgrav wrote: > Robert Watson writes: > > I'm leaning towards (2): debugging simply doesn't apply, and should be > > checked for by the routines. If it were a security check, that might > > imply that you could change the policy to allow such debugging, which you > > can't. Since we would still like ESRCH to be returned for processes in > > jail when attempting to attach to system processes, that means that the > > security check should go first, and the P_SYSTEM check should go second. > > Sure, but doesn't prison_check() take care of ESRCH? I can add the > P_SYSTEM check *after* the prison_check() call in p_candebug(): The question I'm trying to address here is a bit more subtle than that: it's not a question of what we can do, but what we should do. Generally speaking, you might choose to consider security checks of this sort in the following way: there's a certain scope of "valid" requests, and the set of valid requests is going to be limited by a security function. Because there are a number of security failure modes ("you can't even see that it exists" vs. "you can see it, but not touch it"), the ordering and composition of the security checks can be complex. But the question I'm trying to look at here is whether or not, fundamentally, the P_SYSTEM check is a security check. Right now, my leaning is that it is not: checking P_SYSTEM is a validity check because ptrace() and procfs are both definined as allowing the debugging of normal user processes, not system processes. I.e., if you had a system without a security model, you still wouldn't allow debugging of system processes using the user debugger interface. Since the goal of p_candebug() is to impose the general security debugger-related policy, not perform validity checks on arguments, that suggests that the P_SYSTEM check should be present in the service implementation (proc_rwmem() or whatever it's going to be called). Now, that doesn't mean that it shouldn't *also* exist in p_candebug(). You can easily imagine a system security policy that did revolve around protecting system processes--for example, the fact that the jail code restricts the visibility of such processes. Rather than having that implicit on jail membership, it could be explicit, in the form: int p_cansee(...) ... if (p->p_flags & P_SYSTEM) return (ESRCH); ... However, that suggests that if you want to have one in p_candebug(), it should be redundant, and purely an addition to the one in proc_remem(). In fact, I would argue that if there was a P_SYSTEM check in p_candebug(), it should look like this: int p_candebug(...) ... if (p->p_flags & P_SYSTEM) return (EPERM); ... Which is not the desired EINVAL return value. So my opinion is that the P_SYSTEM check should remain in the individual debugging interfaces, and that if redundancy is OK, consider adding a P_SYSTEM->EPERM check in p_candebug(). Interestingly, BTW, this is similar to the issue of read-only mounting of file systems. Right now, my opinion is that if a file system os mounted read-only, that's not a security protection. That's a syntactic function of the mount/file system service. So it should always return EROFS, rather than EPERM, and isn't abstracted into the general security code (via vaccess(), vaccess_acl_posix1e(), vaccess_mac(), et al). An argument to support this is that this is the flag used at the file system level as a validity check to prevent writing to read-only underlying media. Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 8:28: 0 2001 Delivered-To: freebsd-arch@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 4BE3737B403; Sat, 6 Oct 2001 08:27:57 -0700 (PDT) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id RAA65303; Sat, 6 Oct 2001 17:27:56 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Robert Watson Cc: Peter Wemm , arch@FreeBSD.ORG Subject: Re: Removing ptrace(2)'s dependency on procfs(5) References: From: Dag-Erling Smorgrav Date: 06 Oct 2001 17:27:55 +0200 In-Reply-To: Message-ID: Lines: 6 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Thanks for the clarification - I'll leave the P_SYSTEM checks in ptrace() and procfs. DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 18:57:36 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 4C4B437B403; Sat, 6 Oct 2001 18:57:31 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id LAA32651; Sun, 7 Oct 2001 11:57:22 +1000 Date: Sun, 7 Oct 2001 11:56:41 +1000 (EST) From: Bruce Evans X-X-Sender: To: Robert Watson Cc: Dag-Erling Smorgrav , Peter Wemm , Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Message-ID: <20011007114736.D5499-100000@delplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Sat, 6 Oct 2001, Robert Watson wrote: > Well, I guess the decision I was trying to look at was: > > (1) Is it a global security policy that debugging primitives may never be > applied to kernel processes. > > (2) Is it a syntactic property of the debugging primitive that it *cannot* > be applied to kernel processes. I'd like to have separate flags for these attributes. We currently abuse P_SYSTEM for init to prevent debugging and/or swapping of them. This breaks harmless things like /proc/1/map and obfuscates the security checks for init. Most places depend on the P_SYSTEM check to handle init, but at least kern_sig.c still uses both the P_SYSTEM check and a check of init's magic pid. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message From owner-freebsd-arch Sat Oct 6 20:20:25 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id DF23837B403 for ; Sat, 6 Oct 2001 20:20:19 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id NAA03620; Sun, 7 Oct 2001 13:20:11 +1000 Date: Sun, 7 Oct 2001 13:19:29 +1000 (EST) From: Bruce Evans X-X-Sender: To: Dag-Erling Smorgrav Cc: Peter Wemm , Subject: Re: Removing ptrace(2)'s dependency on procfs(5) In-Reply-To: Message-ID: <20011007131723.R6006-100000@delplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 6 Oct 2001, Dag-Erling Smorgrav wrote: > Bruce Evans writes: > > On 5 Oct 2001, Dag-Erling Smorgrav wrote: > > > Dag-Erling Smorgrav writes: > > > > I've put up a new patch that places the prototypes in ptrace.h rather > > > > than add a new header: > > They don't belong in ptrace.h either, since they are used by both procfs > > and ptrace. > > then? That seems reasonable. stopevent() in sys_process.c is already there. large. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message