From owner-freebsd-arch Sat Dec 2 10:16:56 2000 Delivered-To: freebsd-arch@freebsd.org Received: from field.videotron.net (field.videotron.net [205.151.222.108]) by hub.freebsd.org (Postfix) with ESMTP id 5B74437B400 for ; Sat, 2 Dec 2000 10:16:53 -0800 (PST) Received: from modemcable213.3-201-24.mtl.mc.videotron.ca ([24.201.3.213]) by field.videotron.net (Sun Internet Mail Server sims.3.5.1999.12.14.10.29.p8) with ESMTP id <0G4Y0067MDG2K7@field.videotron.net> for arch@FreeBSD.ORG; Sat, 2 Dec 2000 13:16:51 -0500 (EST) Date: Sat, 02 Dec 2000 13:17:36 -0500 (EST) From: Bosko Milekic Subject: Re: zero copy code review In-reply-to: <20001201001619.C10772@panzer.kdm.org> To: "Kenneth D. Merry" Cc: arch@FreeBSD.ORG Message-id: MIME-version: 1.0 Content-type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Fri, 1 Dec 2000, Kenneth D. Merry wrote: > > - Stylistic suggestion: please try to keep things 25x80. :-) > > I try, and I think most of the changes are, except for the NFS stuff. I > didn't reformat that, although I suppose I could. (It irritates me, too.) Ah, that explains it. > In its current incarnation, EXT_DISPOSABLE indicates that the the memory > used in the mbuf can be disposed of -- i.e. removed from the kernel's > virtual address map. The contents aren't disposed of, they're just moved > elsewhere. > > I don't think most of the rest of the mbuf code is setup to deal with the > memory inside a non-external mbuf going away. (Which would be the > potential implication of having EXT_DISPOSABLE be a regular m_flag.) Okay, leaving that exactly the way it is now is The Right Thing To Do (I'm now convinced). > > tiio.h: Are you sure tiio.h belongs in src/sys/sys ? > > Well, it defines the interface for the character device front end for the > ti(4) driver. Usually ioctls and supporting structures go in sys/sys. > Would you suggest another location? No, you're right. > When Bill converted the ti(4) driver from spls to mutexes, I did the same > conversion on my modifications to the driver. Is that sufficient? I'm not > terribly up-to-date on the mutex stuff. > > As for the rest of the code, since it was written pre-mutex, it still has > the spls in the right places. I suppose that they would just need to be > converted to mutexes. (Or is that an overly simplistic way to look at it? :) Well, you really only want to maintain data consistency with the lock. So you'll be looking at protecting your jumbo_kmap lists in the uipc_jumbo.c case with their own lock(s). If you're always looking at both of the lists (inuse and free) at the same time, protecting them with a single lock would be sufficient. For what concerns splvm(), you can leave that as is for now. I've included comments regarding locking in another post, for uipc_jumbo.c As for if_ti, I would have Bill Paul review that. > > Finally, I'd like to > > suggest possibly breaking up some of the diff to smaller chunks, just so > > it is easier to track things down if something does break. With -CURRENT > > changing relatively dramatically now sometimes several times in a single > > day, I think this would be worth it for everybody. > > Heh, well, the big chunk is the Tigon firmware. :) > > Are you suggesting just splitting the diffs out into multiple files, or > actually breaking the changes up? The latter would be rather difficult to > do, I think. I was suggesting breaking some of the changes up, actually, and committing in several chunks (two or three, as opposed to one). But if this is too much of a problem, you don't have to feel obliged to implement the suggestion. > Ken > -- > Kenneth Merry > ken@kdm.org Regards, Bosko Milekic bmilekic@technokratis.com To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message