From owner-cvs-all Sun Apr 2 18:16:36 2000 Delivered-To: cvs-all@freebsd.org Received: from overcee.netplex.com.au (peter1.yahoo.com [208.48.107.4]) by hub.freebsd.org (Postfix) with ESMTP id 7D67837BBA6; Sun, 2 Apr 2000 18:16:28 -0700 (PDT) (envelope-from peter@netplex.com.au) Received: from netplex.com.au (localhost [127.0.0.1]) by overcee.netplex.com.au (Postfix) with ESMTP id 6E7AE1CD7; Sun, 2 Apr 2000 18:16:27 -0700 (PDT) (envelope-from peter@netplex.com.au) X-Mailer: exmh version 2.1.1 10/15/1999 To: Matthew Dillon Cc: Poul-Henning Kamp , cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern subr_devstat.c src/sys/ufs/ufs ufs_disksubr.c src/sys/sys buf.h devicestat.h disklabel.h In-Reply-To: Message from Matthew Dillon of "Sun, 02 Apr 2000 16:26:37 PDT." <200004022326.QAA51447@apollo.backplane.com> Date: Sun, 02 Apr 2000 18:16:27 -0700 From: Peter Wemm Message-Id: <20000403011627.6E7AE1CD7@overcee.netplex.com.au> Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Matthew Dillon wrote: > > : > :In message <200004022223.PAA51020@apollo.backplane.com>, Matthew Dillon writ es: > : > :> b_data is integral to buffer cache KVA management & operation and > :> should be in the main struct buf structure, not the bio structure. > : > :No, b_data is (currently) also a property of the I/O request. > > No it isn't. b_data is the property of the buffer cache. If you > screw with it, you blow up the buffer cache. Any device driver that > screws with it must restore it prior to reentering the buffer cache > code. [..] > If you want you can duplicate b_data ... that is, have the buffer cache > managed KVA be a field in struct buf and copy it into a new field in the > bio which the device drivers use (and get rid of the save field that > the device drivers currently use to save/restore b_data). But that isn't > what you've done. I think this is probably the better thing to do - ie: duplicate it and also the B_ERROR flag. As you point out, brelse() uses it. However, it's also part of the driver<->bio interaction. This is part of the problem with doing this incrementally and trying to keep disk drivers working with backwards compatability. Consider: B_ERROR is used by the buffer cache to clean up after transaction failures (eg: preserving data after write failures for retry) etc and/or for reporting errno's. But B_ERROR was also a way the driver reported the error to the bio layer. What I suspect is the "correct" answer is to duplicate it so that there is an error reporting mechanism between the IO system and drivers, and any error returns are propagated back up to b_flags |= B_ERROR for the buffer cache so that it can do the right thing. But doing that with incremental code changes is a pain because presently it's all #defined for backwards compatability. I'm sure I'm not describing my point clearly, but it's worth a try. Putting it differently (assuming bio != buffer cache terminology) - bio_data is a property of a 'bio' transaction - BIO_ERROR is an error return mechanism of a bio <-> driver transaction - A bio transaction may not necessarily be on a "buffer" - it could be on anything, including a page, a block of data, etc. but also: - b_data is a property of the buffer cache - B_ERROR is also an attribute of an individual buffer in the buffer cache. - buffer cache 'buffers' are not part of the IO request subsystem. I think Poul-Henning is on the right track, and there is plenty of time to get this "right". Temporarily breaking layering so that this can be done incrementally is probably OK *providing* that the big picture is kept in mind and that the layering is fixed. IMHO, redoing the device driver bio transaction layer to take 'struct bio' instead of 'struct buf' is the bigger footprint change and is a prerequisite of splitting the bio and buf structures - at the point that bio and buf split properly then we have to deal with and restore b_data and B_ERROR and propagate them back up properly. Does that make any sense? (It's times like this I wish I hadn't given up coffee!) Cheers, -Peter To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message