From owner-freebsd-arch Wed Jan 22 7: 1:34 2003 Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D3C6F37B401 for ; Wed, 22 Jan 2003 07:01:31 -0800 (PST) Received: from tesla.distributel.net (nat.MTL.distributel.NET [66.38.181.24]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1E20643F5B for ; Wed, 22 Jan 2003 07:01:31 -0800 (PST) (envelope-from bmilekic@unixdaemons.com) Received: (from bmilekic@localhost) by tesla.distributel.net (8.11.6/8.11.6) id h0MF2r576494; Wed, 22 Jan 2003 10:02:53 -0500 (EST) (envelope-from bmilekic@unixdaemons.com) Date: Wed, 22 Jan 2003 10:02:53 -0500 From: Bosko Milekic To: "M. Warner Losh" Cc: bright@mu.org, arch@FreeBSD.ORG Subject: Re: M_ flags summary. Message-ID: <20030122100253.A76397@unixdaemons.com> References: <20030122023246.GP42333@elvis.mu.org> <20030121224148.A75236@unixdaemons.com> <20030121.222025.101592442.imp@bsdimp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <20030121.222025.101592442.imp@bsdimp.com>; from imp@bsdimp.com on Tue, Jan 21, 2003 at 10:20:25PM -0700 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, Jan 21, 2003 at 10:20:25PM -0700, M. Warner Losh wrote: > In message: <20030121224148.A75236@unixdaemons.com> > Bosko Milekic writes: > : I think that defining M_TRYWAIT and M_WAITOK to 0 for KLD_MODULES is > : fine but I do not think that defining them to anything other than 0 is > : fine just so that we could add that KASSERT() that Warren suggested in > : the allocation code. As you point out, defining it to anything other > : than 0 would actually break ABI compatibility. Defining it to 0 for > : KLD_MODULES would preserve both API and ABI compatibility for those > : who actually care. Certainly, both M_TRYWAIT and M_WAITOK would have > : to be defined in order to maintain full backwards-API compatibility. > > Actually, I think that we shouldn't define them to be 0 for modules. > Instead, we should define them to the new values. However, we should > accpet '0' with the old meaning for a while (maybe with a printf). > There are going to be enough ABI changes between 5.0 and 5.branch that > worrying about this one is likely not worth the effort to do special > things for the modules. It isn't going to be too much longer before > it becomes impossible for 5.0-RELEASE compiled modules to not operate > with 5.0-CURRENT. Changing the value to something other than zero has the following problems: 1. Breaks ABI compatibility (but granted this is fine as long as it's done with good reason, and you have to prove that it's good reason beyond just 'this is the way I think it should be done') 2. Gives the impression that it's OK if just any code calls the allocators requesting either the wait behavior or the dont-wait behavior. This is the biggest point that you have to argue and if you can convince us here you can convince us that your change is worth breaking ABI compatibility for. Now, from what I gather, your argument for re-introducing the M_WAIT{,OK} flags and defining them to something other than zero is: 1. It allows us to error-check in the allocator code and make sure that the caller is passing in one or the other and be able to provide some sort of feedback to the programmer (either via a printf, or whatever) letting him know that he's made a mistake. 2. You think that the caller should always be allowed to specify whether or not he is willing to wait. This is basically the counter-part of (2) above. I'm going to argue why I think (1) is not worth it first. The claim that the added error reporting is worth it assumes that the caller is unknowingly making a mistake. How do you expect that this will happen? Your claim is that if the programmer calls the allocator without specifying either M_WAIT{,OK} or M_{NO,DONT}WAIT, that the allocator should spew out a warning, at the least. My claim is that unless the programmer passes in M_{NO,DONT}WAIT, the allocator should act according to default behavior. This really ties into (2) above. You're saying that any code needs to specify explicitly whether it wants the wait or dont-wait behavior. I'm saying that code should only explicitly request _not_ waiting. My argument is that only interrupt code and broken code should request the dontwait behavior. If you don't agree, then this is what you have to argue. I think that non-interrupt code should always be willing to try waiting merely because it can. Why would we take the trouble to design a system that allows concurrency in the kernel if we then proceed to write code that doesn't take advantage of it? The only exception is broken code which, possibly due to locking reasons, cannot wait (and we will be able to catch such code later on by grepping for the M_{NO,DONT}WAIT flags) and interrupt code which, due to our design, cannot sleep. So, basically what I'm saying is that the programmer is NEVER making a mistake unless he's calling the allocator code from an ISR without explicitly requesting the dontwait behavior. And if he makes that mistake, we'll leave things to blow right up once the ISR tries to msleep (we'll trap that problem in the scheduler code, not in the allocator code). The one exception, again, is if the programmer needs to specifically request the dontwait behavior because of locking problems he has in his code but in that case your idea of defining M_{,TRY}WAIT to something other than 0 isn't going to help him much either (he would spot the printf, and instead of taking the correct approach to fixing his problem, i.e. fixing the locking issue, he would just pass the flag that silences the printf()). Therefore, I believe that the default behavior in the system should be to always try to wait, at the very least, unless the programmer specifically requests that no waiting is performed. > I think we need to go fartehr than Alfred[*] is wanting to go, but > until I post a patch I'm going to be quiet. > > Warner Regards, -- Bosko Milekic * bmilekic@unixdaemons.com * bmilekic@FreeBSD.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message