Re: B_WRITE cleanup patch, please test!
Poul-Henning Kamp wrote: In message [EMAIL PROTECTED], Matthew Dillon writes: I think the biggest win in regards to being able to arbitrarily stack devices is to NOT attempt to forward struct buf's between devices when non-trivial manipulation is required, and instead to make struct buf's cheap enough that a device can simply allocate a new one and copy the appropriate fields. In particular I really hate all the various b_*blkno fields. b_lblkno, b_blkno, and b_pblkno. It is precisely due to the existance of these hacks that arbitrary device stacking is difficult. This is basically what the stuff I'm doing addresses. I have been advocating since 1991 that the memory involved with IO should be referenced as a vector of page/offset/length triplets (physical addresses), and that all drivers should take that as input. IF he driver needs to do programmed IO only then should it map the page into KV space. -- Poul-Henning Kamp FreeBSD coreteam member [EMAIL PROTECTED] "Real hackers run -current on their laptop." FreeBSD -- It will take a long time before progress goes too far! -- __--_|\ Julian Elischer / \ [EMAIL PROTECTED] ( OZ) World tour 2000 --- X_.---._/ presently in: Perth v To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: B_WRITE cleanup patch, please test!
I think the biggest win in regards to being able to arbitrarily stack devices is to NOT attempt to forward struct buf's between devices when non-trivial manipulation is required, and instead to make struct buf's cheap enough that a device can simply allocate a new one and copy the appropriate fields. Here I am talking about situations where devices need callbacks (making forwarding impossible), need to split or combine requests, or do other non-trivial things. In particular I really hate all the various b_*blkno fields. b_lblkno, b_blkno, and b_pblkno. It is precisely due to the existance of these hacks that arbitrary device stacking is difficult. The key to making a struct buf cheap is to provide an I/O path that does not require the b_data KVA mapping. Once we provide this path, I think everything else will fall into place quite neatly. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: B_WRITE cleanup patch, please test!
In message [EMAIL PROTECTED], Matthew Dillon writes: I think the biggest win in regards to being able to arbitrarily stack devices is to NOT attempt to forward struct buf's between devices when non-trivial manipulation is required, and instead to make struct buf's cheap enough that a device can simply allocate a new one and copy the appropriate fields. In particular I really hate all the various b_*blkno fields. b_lblkno, b_blkno, and b_pblkno. It is precisely due to the existance of these hacks that arbitrary device stacking is difficult. This is basically what the stuff I'm doing addresses. -- Poul-Henning Kamp FreeBSD coreteam member [EMAIL PROTECTED] "Real hackers run -current on their laptop." FreeBSD -- It will take a long time before progress goes too far! To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: B_WRITE cleanup patch, please test!
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 [EMAIL PROTECTED] "Real hackers run -current on their laptop." FreeBSD -- It will take a long time before progress goes too far! To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message -- __--_|\ Julian Elischer / \ [EMAIL PROTECTED] ( OZ) World tour 2000 --- X_.---._/ presently in: Perth v To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: B_WRITE cleanup patch, please test!
In message [EMAIL PROTECTED], Julian Elischer writes: 1/ I think the removal of IPSEC from LINT is by mistake in this patch yes, that is a mistake. 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 I decided to use binary values, because there is a simple arithmetic test for "exactly one bit set" (See BUF_STRATEGY). I wanted to catch as many bogus cases of B_WRITE zeroness being abused as possible, ie, people initializing b_flags = 0 knowing that would give them a write. 4/ does this produce compatibility problems with any 3rd party code? This is 5.0-CURRENT, they will have to adopt to much worse things before it becomes 5.0-RELEASE. -- Poul-Henning Kamp FreeBSD coreteam member [EMAIL PROTECTED] "Real hackers run -current on their laptop." FreeBSD -- It will take a long time before progress goes too far! To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
B_WRITE cleanup patch, please test!
http://phk.freebsd.dk/misc/b_iocmd.patch 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 [EMAIL PROTECTED] "Real hackers run -current on their laptop." FreeBSD -- It will take a long time before progress goes too far! To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: B_WRITE cleanup patch, please test!
It is most ironic that of course you were the loudest supporter of SOS when he ripped out all my code that did EXACTLY all this. (I might add that this was done without any warning to me. The The commit messages being my first notice). It was fully working and quite a few people were running it at the time. It looks like what you are suggesting is pretty much exactly what I had running so I certainly agree with it. I just am still still smarting from the fact that it was apparently deleted simply because it was written by me. It certainly ended most of my direct involvement with freebsd other than through my work so I guess it had it's desired effect. Poul-Henning Kamp wrote: http://phk.freebsd.dk/misc/b_iocmd.patch 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 [EMAIL PROTECTED] "Real hackers run -current on their laptop." FreeBSD -- It will take a long time before progress goes too far! Unless of course it wasn't written in Denmark To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message -- __--_|\ Julian Elischer / \ [EMAIL PROTECTED] ( OZ) World tour 2000 --- X_.---._/ presently in: Perth v To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: B_WRITE cleanup patch, please test!
Poul-Henning Kamp wrote: This patch is the first step towards the stackable BIO system as sketched out on http://www.freebsd.org/~phk/Geom/ you don't mention how you plan to get around the problem that arbitrarily stacking devices means arbitrarily allocating minor numbers. I used devfs to do this. -- __--_|\ Julian Elischer / \ [EMAIL PROTECTED] ( OZ) World tour 2000 --- X_.---._/ presently in: Perth v To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: B_WRITE cleanup patch, please test!
It seems Julian Elischer wrote: It is most ironic that of course you were the loudest supporter of SOS when he ripped out all my code that did EXACTLY all this. (I might add that this was done without any warning to me. The The commit messages being my first notice). It was fully working and quite a few people were running it at the time. I think there are at least two wildly different oppinions on that Julian, besides this was not my/phk's decision alone. It looks like what you are suggesting is pretty much exactly what I had running so I certainly agree with it. I just am still still smarting from the fact that it was apparently deleted simply because it was written by me. It certainly ended most of my direct involvement with freebsd other than through my work so I guess it had it's desired effect. I'll let phk comment on that one :) -Søren To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: B_WRITE cleanup patch, please test!
This patch is the first step towards the stackable BIO system as sketched out on http://www.freebsd.org/~phk/Geom/ Please test review. There's no code to test- it's just a sketch. It's fine as a start, but it's important that you clarify the role of node device drivers in informing the geometry device about what's what (to get information about physical limits) and how errors are to be flagged (if an out of range request comes floating thru). Presumably the latter is just marking a transaction with B_ERROR and setting b_errno to something specific that would say that the block is out of range (insert argument over 'correct' errno here), and that the range involved is the physical device limit (and what about commands that overlap the end of the device?). Presumably the former, if to give the geometry stack something worthwhile to chew on, can handle the case of devices that resize and devices where, unlike most devices currently, geometry really does matter. -matt To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: B_WRITE cleanup patch, please test!
On Mon, 13 Mar 2000, Matthew Jacob wrote:
This patch is the first step towards the stackable BIO system as
sketched out on http://www.freebsd.org/~phk/Geom/
Please test review.
{ as Poul reminded me, I missed the header line that had the patch - sorry
about that }
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
