Date: Thu, 16 Mar 2000 07:27:38 -0800 From: Julian Elischer <julian@elischer.org> To: Poul-Henning Kamp <phk@freebsd.org> Cc: current@freebsd.org Subject: Re: B_WRITE cleanup patch, please test! Message-ID: <38D0FD6A.167EB0E7@elischer.org> References: <20502.952948995@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
Poul-Henning Kamp wrote: > > http://phk.freebsd.dk/misc/b_iocmd.patch I concur that these patches represent effectlively a mechanical edit of the sources to produce effectively the same functionality as before. I have a couple of points which I would like to discuss. These are not complaints, just discussion points 1/ I think the removal of IPSEC from LINT is by mistake in this patch 2/ The change of separating buffer management from IO management is long overdue and the introduction of b_iocmd is a good first step for this. The selection of values is in my mind arguable either way. As the field is an 'op-code' I would have chosen the values to be: #define BIO_NOP 0 #define BIO_READ 1 #define BIO_WRITE 2 #define BIO_DELETE 3 #define BIO_OP 0x03 /* The bit mask for the basic operation*/ I notice that you Strategy macro checks for illegal values, but wonder whether it might be better to make illegal values 'impossible'. This is a pure style/scope question. 3/ The removal of B_CALL, and the use of the non-null value of the vector is an editing change only and not really controversial. It does simplify the question of whether the field belongs in the IO control part of the buffer control part. 4/ does this produce compatibility problems with any 3rd party code? Over all this edit is a functional not and a move in the right direction from a scope point of view. Unless someone else can think of reasons, I have nothing against it. People I'd like to see as having looked at it however, include Matt and Kirk. > > B_WRITE is bogusly defined as zero, which is a perfect candidate > for coding and logic mistakes, we saw the most recent victim of > this bogosity as recently as a few days ago. > > This patch moves the "io-command" aspect of the b_flags into a new > struct buf field called b_iocmd. > > This patch is the first step towards the stackable BIO system as > sketched out on http://www.freebsd.org/~phk/Geom/ > > Please test & review. > > -- > Poul-Henning Kamp FreeBSD coreteam member > phk@FreeBSD.ORG "Real hackers run -current on their laptop." > FreeBSD -- It will take a long time before progress goes too far! > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-current" in the body of the message -- __--_|\ Julian Elischer / \ julian@elischer.org ( OZ ) World tour 2000 ---> X_.---._/ presently in: Perth v To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?38D0FD6A.167EB0E7>