Date: Sat, 02 Dec 2000 13:17:36 -0500 (EST) From: Bosko Milekic <bmilekic@technokratis.com> To: "Kenneth D. Merry" <ken@kdm.org> Cc: arch@FreeBSD.ORG Subject: Re: zero copy code review Message-ID: <Pine.BSF.4.21.0012021308040.91662-100000@jehovah.technokratis.com> In-Reply-To: <20001201001619.C10772@panzer.kdm.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0012021308040.91662-100000>